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 bdeb96f5 fix: delete data plane config when a route leaves a managed
Gateway (#2834)
bdeb96f5 is described below
commit bdeb96f571af161d9ede0924044f7247ac64b671
Author: Johannes Engler <[email protected]>
AuthorDate: Wed Sep 9 09:37:30 2026 +0200
fix: delete data plane config when a route leaves a managed Gateway (#2834)
---
internal/adc/client/client.go | 7 +-
internal/controller/grpcroute_controller.go | 18 ++-
internal/controller/httproute_controller.go | 23 ++-
internal/controller/httproute_controller_test.go | 111 +++++++++++++++
internal/controller/tcproute_controller.go | 18 ++-
internal/controller/tlsroute_controller.go | 18 ++-
internal/controller/udproute_controller.go | 18 ++-
internal/controller/utils.go | 17 ++-
internal/controller/utils_parentref_test.go | 84 ++++++++++-
internal/provider/apisix/provider.go | 10 +-
internal/provider/apisix/provider_test.go | 67 +++++++++
test/e2e/gatewayapi/httproute.go | 170 +++++++++++++++++++++++
12 files changed, 541 insertions(+), 20 deletions(-)
diff --git a/internal/adc/client/client.go b/internal/adc/client/client.go
index b3db60ca..e80d1b27 100644
--- a/internal/adc/client/client.go
+++ b/internal/adc/client/client.go
@@ -240,9 +240,10 @@ func (c *Client) Delete(ctx context.Context, args Task)
error {
return c.applySync(ctx, args, delta)
}
-func (c *Client) DeleteConfig(ctx context.Context, args Task) error {
- _, err := c.applyStoreChanges(args, true)
- return err
+// DeleteConfig removes the stored configuration for args.Key and reports what
+// it removed, so callers can skip a data plane sync when the key held nothing.
+func (c *Client) DeleteConfig(ctx context.Context, args Task) (StoreDelta,
error) {
+ return c.applyStoreChanges(args, true)
}
func (c *Client) Validate(ctx context.Context, task Task) error {
diff --git a/internal/controller/grpcroute_controller.go
b/internal/controller/grpcroute_controller.go
index dc3d0e03..bbc45265 100644
--- a/internal/controller/grpcroute_controller.go
+++ b/internal/controller/grpcroute_controller.go
@@ -181,12 +181,28 @@ func (r *GRPCRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
msg: "Route is accepted",
}
- gateways, err := ParseRouteParentRefs(ctx, r.Client, r.Log, gr,
gr.Spec.ParentRefs)
+ gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client,
r.Log, gr, gr.Spec.ParentRefs)
if err != nil {
return ctrl.Result{}, err
}
if len(gateways) == 0 {
+ if unresolvedParents {
+ // See the HTTPRoute reconciler: an unresolvable
parentRef leaves ownership
+ // unknown rather than disproven, so the data plane
must be left alone.
+ return ctrl.Result{}, nil
+ }
+ // See the HTTPRoute reconciler: a route that no longer
references a
+ // Gateway managed by this controller must have its previously
pushed
+ // configuration removed, or the data plane keeps serving it.
+ gr.TypeMeta = metav1.TypeMeta{
+ Kind: KindGRPCRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, gr); err != nil {
+ r.Log.Error(err, "failed to delete grpcroute",
"grpcroute", gr)
+ return ctrl.Result{}, err
+ }
return ctrl.Result{}, nil
}
diff --git a/internal/controller/httproute_controller.go
b/internal/controller/httproute_controller.go
index f721ebd2..26b94683 100644
--- a/internal/controller/httproute_controller.go
+++ b/internal/controller/httproute_controller.go
@@ -163,12 +163,33 @@ func (r *HTTPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
msg: "Route is accepted",
}
- gateways, err := ParseRouteParentRefs(ctx, r.Client, r.Log, hr,
hr.Spec.ParentRefs)
+ gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client,
r.Log, hr, hr.Spec.ParentRefs)
if err != nil {
return ctrl.Result{}, err
}
if len(gateways) == 0 {
+ if unresolvedParents {
+ // A missing Gateway or GatewayClass leaves ownership
unknown rather than
+ // disproven. GatewayClass is cluster-scoped, so while
one is absent every
+ // route under it resolves empty and deleting would
drain the data plane.
+ return ctrl.Result{}, nil
+ }
+ // The route does not reference any Gateway managed by this
controller.
+ // It may have referenced one before, e.g. when its parentRefs
are
+ // repointed at a Gateway belonging to another GatewayClass, so
the
+ // configuration a previous reconcile pushed has to be removed.
Without
+ // this the data plane keeps serving the route indefinitely.
+ // Provider.Delete derives the resource labels from the object
Kind, which
+ // is empty on objects read through the client.
+ hr.TypeMeta = metav1.TypeMeta{
+ Kind: KindHTTPRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, hr); err != nil {
+ r.Log.Error(err, "failed to delete httproute",
"httproute", hr)
+ return ctrl.Result{}, err
+ }
return ctrl.Result{}, nil
}
diff --git a/internal/controller/httproute_controller_test.go
b/internal/controller/httproute_controller_test.go
new file mode 100644
index 00000000..00876c02
--- /dev/null
+++ b/internal/controller/httproute_controller_test.go
@@ -0,0 +1,111 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package controller
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ "github.com/go-logr/logr"
+ "github.com/stretchr/testify/assert"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ k8stypes "k8s.io/apimachinery/pkg/types"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/fake"
+ gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ "github.com/apache/apisix-ingress-controller/internal/manager/readiness"
+)
+
+type noopReadier struct {
+ readiness.ReadinessManager
+}
+
+func (noopReadier) Done(client.Object, k8stypes.NamespacedName) {}
+
+// TestHTTPRouteReconcile_EmptyGateways covers the two ways a route resolves
to no
+// Gateway of this controller. Only a resolved parent naming another controller
+// proves the route is not ours; an unresolvable parent must leave the data
plane
+// alone, since a missing cluster-scoped GatewayClass empties the list for
every
+// route under it at once.
+func TestHTTPRouteReconcile_EmptyGateways(t *testing.T) {
+ scheme := parentRefTestScheme(t)
+
+ route := &gatewayv1.HTTPRoute{
+ ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "r"},
+ Spec: gatewayv1.HTTPRouteSpec{
+ CommonRouteSpec: gatewayv1.CommonRouteSpec{
+ ParentRefs: []gatewayv1.ParentReference{{Name:
"gw"}},
+ },
+ },
+ }
+ gw := func(class string) *gatewayv1.Gateway {
+ return &gatewayv1.Gateway{
+ ObjectMeta: metav1.ObjectMeta{Namespace: "default",
Name: "gw"},
+ Spec: gatewayv1.GatewaySpec{
+ GatewayClassName: gatewayv1.ObjectName(class),
+ Listeners: []gatewayv1.Listener{
+ {Name: "http", Port: 80, Protocol:
gatewayv1.HTTPProtocolType},
+ },
+ },
+ }
+ }
+
+ for _, tc := range []struct {
+ name string
+ objects []client.Object
+ wantDelete bool
+ }{
+ {
+ name: "gatewayclass of another controller",
+ objects: []client.Object{route, gw("other"),
&gatewayv1.GatewayClass{
+ ObjectMeta: metav1.ObjectMeta{Name: "other"},
+ Spec:
gatewayv1.GatewayClassSpec{ControllerName: "example.com/other-controller"},
+ }},
+ wantDelete: true,
+ },
+ {
+ name: "gatewayclass missing",
+ objects: []client.Object{route, gw("gone")},
+ wantDelete: false,
+ },
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ cli :=
fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.objects...).Build()
+ prov := &recordingProvider{}
+ r := &HTTPRouteReconciler{
+ Client: cli,
+ Scheme: scheme,
+ Log: logr.Discard(),
+ Provider: prov,
+ Readier: noopReadier{},
+ }
+
+ ctx, cancel :=
context.WithTimeout(context.Background(), 10*time.Second)
+ defer cancel()
+ _, err := r.Reconcile(ctx, ctrl.Request{
+ NamespacedName:
k8stypes.NamespacedName{Namespace: "default", Name: "r"},
+ })
+ assert.NoError(t, err)
+ assert.Equal(t, tc.wantDelete, len(prov.deleted) == 1,
+ "unexpected data plane delete, deletes=%d",
len(prov.deleted))
+ })
+ }
+}
diff --git a/internal/controller/tcproute_controller.go
b/internal/controller/tcproute_controller.go
index 927244cb..4cf16ebe 100644
--- a/internal/controller/tcproute_controller.go
+++ b/internal/controller/tcproute_controller.go
@@ -273,12 +273,28 @@ func (r *TCPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
msg: "Route is accepted",
}
- gateways, err := ParseRouteParentRefs(ctx, r.Client, r.Log, tr,
tr.Spec.ParentRefs)
+ gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client,
r.Log, tr, tr.Spec.ParentRefs)
if err != nil {
return ctrl.Result{}, err
}
if len(gateways) == 0 {
+ if unresolvedParents {
+ // See the HTTPRoute reconciler: an unresolvable
parentRef leaves ownership
+ // unknown rather than disproven, so the data plane
must be left alone.
+ return ctrl.Result{}, nil
+ }
+ // See the HTTPRoute reconciler: a route that no longer
references a
+ // Gateway managed by this controller must have its previously
pushed
+ // configuration removed, or the data plane keeps serving it.
+ tr.TypeMeta = metav1.TypeMeta{
+ Kind: KindTCPRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, tr); err != nil {
+ r.Log.Error(err, "failed to delete tcproute",
"tcproute", tr)
+ return ctrl.Result{}, err
+ }
return ctrl.Result{}, nil
}
diff --git a/internal/controller/tlsroute_controller.go
b/internal/controller/tlsroute_controller.go
index e2d62453..c9df0e62 100644
--- a/internal/controller/tlsroute_controller.go
+++ b/internal/controller/tlsroute_controller.go
@@ -273,12 +273,28 @@ func (r *TLSRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
msg: "Route is accepted",
}
- gateways, err := ParseRouteParentRefs(ctx, r.Client, r.Log, tr,
tr.Spec.ParentRefs)
+ gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client,
r.Log, tr, tr.Spec.ParentRefs)
if err != nil {
return ctrl.Result{}, err
}
if len(gateways) == 0 {
+ if unresolvedParents {
+ // See the HTTPRoute reconciler: an unresolvable
parentRef leaves ownership
+ // unknown rather than disproven, so the data plane
must be left alone.
+ return ctrl.Result{}, nil
+ }
+ // See the HTTPRoute reconciler: a route that no longer
references a
+ // Gateway managed by this controller must have its previously
pushed
+ // configuration removed, or the data plane keeps serving it.
+ tr.TypeMeta = metav1.TypeMeta{
+ Kind: types.KindTLSRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, tr); err != nil {
+ r.Log.Error(err, "failed to delete tlsroute",
"tlsroute", tr)
+ return ctrl.Result{}, err
+ }
return ctrl.Result{}, nil
}
diff --git a/internal/controller/udproute_controller.go
b/internal/controller/udproute_controller.go
index ffb43644..2c8a910b 100644
--- a/internal/controller/udproute_controller.go
+++ b/internal/controller/udproute_controller.go
@@ -273,12 +273,28 @@ func (r *UDPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
msg: "Route is accepted",
}
- gateways, err := ParseRouteParentRefs(ctx, r.Client, r.Log, tr,
tr.Spec.ParentRefs)
+ gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client,
r.Log, tr, tr.Spec.ParentRefs)
if err != nil {
return ctrl.Result{}, err
}
if len(gateways) == 0 {
+ if unresolvedParents {
+ // See the HTTPRoute reconciler: an unresolvable
parentRef leaves ownership
+ // unknown rather than disproven, so the data plane
must be left alone.
+ return ctrl.Result{}, nil
+ }
+ // See the HTTPRoute reconciler: a route that no longer
references a
+ // Gateway managed by this controller must have its previously
pushed
+ // configuration removed, or the data plane keeps serving it.
+ tr.TypeMeta = metav1.TypeMeta{
+ Kind: KindUDPRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, tr); err != nil {
+ r.Log.Error(err, "failed to delete udproute",
"udproute", tr)
+ return ctrl.Result{}, err
+ }
return ctrl.Result{}, nil
}
diff --git a/internal/controller/utils.go b/internal/controller/utils.go
index db6894f4..cf5167b4 100644
--- a/internal/controller/utils.go
+++ b/internal/controller/utils.go
@@ -324,14 +324,21 @@ func parentRefTargetsListenerExplicitly(parentRef
gatewayv1.ParentReference) boo
return parentRef.Port != nil
}
+// ParseRouteParentRefs resolves the parentRefs of a route to the Gateways this
+// controller manages. The second return value reports that at least one
+// parentRef could not be resolved, because its Gateway or that Gateway's
+// GatewayClass does not exist. An empty gateway list then means "ownership
+// unknown", not "owned by another controller", and callers must not act on the
+// route's data plane configuration.
func ParseRouteParentRefs(
ctx context.Context,
mgrc client.Client,
log logr.Logger,
route client.Object,
parentRefs []gatewayv1.ParentReference,
-) ([]RouteParentRefContext, error) {
+) ([]RouteParentRefContext, bool, error) {
gateways := make([]RouteParentRefContext, 0)
+ unresolved := false
for _, parentRef := range parentRefs {
namespace := route.GetNamespace()
if parentRef.Namespace != nil {
@@ -349,9 +356,10 @@ func ParseRouteParentRefs(
Name: name,
}, &gateway); err != nil {
if client.IgnoreNotFound(err) == nil {
+ unresolved = true
continue
}
- return nil, fmt.Errorf("failed to retrieve gateway for
route: %w", err)
+ return nil, false, fmt.Errorf("failed to retrieve
gateway for route: %w", err)
}
gatewayClass := gatewayv1.GatewayClass{}
@@ -359,9 +367,10 @@ func ParseRouteParentRefs(
Name: string(gateway.Spec.GatewayClassName),
}, &gatewayClass); err != nil {
if client.IgnoreNotFound(err) == nil {
+ unresolved = true
continue
}
- return nil, fmt.Errorf("failed to retrieve gatewayclass
for gateway: %w", err)
+ return nil, false, fmt.Errorf("failed to retrieve
gatewayclass for gateway: %w", err)
}
if string(gatewayClass.Spec.ControllerName) !=
config.ControllerConfig.ControllerName {
@@ -499,7 +508,7 @@ func ParseRouteParentRefs(
}
}
- return gateways, nil
+ return gateways, unresolved, nil
}
// reuseUnchangedListenerStatus keeps the previously published status when
diff --git a/internal/controller/utils_parentref_test.go
b/internal/controller/utils_parentref_test.go
index 1e74f0ce..4a6bf3b3 100644
--- a/internal/controller/utils_parentref_test.go
+++ b/internal/controller/utils_parentref_test.go
@@ -27,6 +27,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/ptr"
+ "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -94,7 +95,7 @@ func TestParseRouteParentRefs_ReasonOrderIndependent(t
*testing.T) {
cli := fake.NewClientBuilder().WithScheme(scheme).
WithObjects(newParentRefGatewayClass(),
gw).Build()
- got, err := ParseRouteParentRefs(context.Background(),
cli, logr.Discard(), route,
+ got, _, err :=
ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route,
[]gatewayv1.ParentReference{{Name: "gw"}})
require.NoError(t, err)
require.Len(t, got, 1)
@@ -145,7 +146,7 @@ func TestParseRouteParentRefs_ExplicitListenerMatch(t
*testing.T) {
port8080 := gatewayv1.PortNumber(8080)
t.Run("invalid explicit ref is not satisfied by another gateway's
listener", func(t *testing.T) {
- got, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
+ got, _, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
[]gatewayv1.ParentReference{
// Explicit sectionName "web" on gw-a, which
has no such listener.
{Name: "gw-a", SectionName: &webSection},
@@ -171,7 +172,7 @@ func TestParseRouteParentRefs_ExplicitListenerMatch(t
*testing.T) {
})
t.Run("explicit sectionName on the owning gateway is an explicit
match", func(t *testing.T) {
- got, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
+ got, _, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
[]gatewayv1.ParentReference{{Name: "gw-b", SectionName:
&webSection}})
require.NoError(t, err)
require.Len(t, got, 1)
@@ -180,7 +181,7 @@ func TestParseRouteParentRefs_ExplicitListenerMatch(t
*testing.T) {
})
t.Run("explicit port on the owning gateway is an explicit match",
func(t *testing.T) {
- got, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
+ got, _, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
[]gatewayv1.ParentReference{{Name: "gw-b", Port:
&port8080}})
require.NoError(t, err)
require.Len(t, got, 1)
@@ -189,7 +190,7 @@ func TestParseRouteParentRefs_ExplicitListenerMatch(t
*testing.T) {
})
t.Run("implicit ref is not an explicit match", func(t *testing.T) {
- got, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
+ got, _, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
[]gatewayv1.ParentReference{{Name: "gw-b"}})
require.NoError(t, err)
require.Len(t, got, 1)
@@ -222,7 +223,7 @@ func TestParseRouteParentRefs_ConflictingTLSModePort(t
*testing.T) {
cli := fake.NewClientBuilder().WithScheme(scheme).
WithObjects(newParentRefGatewayClass(), gw).Build()
- got, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
+ got, _, err := ParseRouteParentRefs(context.Background(), cli,
logr.Discard(), route,
[]gatewayv1.ParentReference{{Name: "gw"}})
require.NoError(t, err)
require.Len(t, got, 1)
@@ -231,3 +232,74 @@ func TestParseRouteParentRefs_ConflictingTLSModePort(t
*testing.T) {
"route must not attach to a conflicting-tls-mode port")
assert.Equal(t, string(gatewayv1.RouteReasonNotAllowedByListeners),
cond.Reason)
}
+
+// TestParseRouteParentRefs_UnresolvedParents verifies the second return value
+// distinguishes "no parentRef named this controller" from "a parentRef could
not
+// be resolved". Callers delete data plane configuration on an empty gateway
list,
+// which is only correct in the first case: a GatewayClass is cluster-scoped,
so
+// while one is missing every route under it resolves to an empty list at once.
+func TestParseRouteParentRefs_UnresolvedParents(t *testing.T) {
+ scheme := parentRefTestScheme(t)
+
+ route := &gatewayv1.HTTPRoute{
+ ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "r"},
+ }
+ listener := gatewayv1.Listener{Name: "http", Port: 80, Protocol:
gatewayv1.HTTPProtocolType}
+ gw := func(name, class string) *gatewayv1.Gateway {
+ return &gatewayv1.Gateway{
+ ObjectMeta: metav1.ObjectMeta{Namespace: "default",
Name: name},
+ Spec: gatewayv1.GatewaySpec{
+ GatewayClassName: gatewayv1.ObjectName(class),
+ Listeners:
[]gatewayv1.Listener{listener},
+ },
+ }
+ }
+ foreignClass := &gatewayv1.GatewayClass{
+ ObjectMeta: metav1.ObjectMeta{Name: "other"},
+ Spec: gatewayv1.GatewayClassSpec{ControllerName:
"example.com/other-controller"},
+ }
+
+ for _, tc := range []struct {
+ name string
+ objects []client.Object
+ parentRef string
+ wantGateways int
+ wantUnresolved bool
+ }{
+ {
+ name: "gateway missing",
+ objects:
[]client.Object{newParentRefGatewayClass()},
+ parentRef: "gw",
+ wantUnresolved: true,
+ },
+ {
+ name: "gatewayclass missing",
+ objects: []client.Object{gw("gw", "apisix")},
+ parentRef: "gw",
+ wantUnresolved: true,
+ },
+ {
+ name: "gateway of another controller",
+ objects: []client.Object{foreignClass, gw("gw",
"other")},
+ parentRef: "gw",
+ wantUnresolved: false,
+ },
+ {
+ name: "gateway of this controller",
+ objects:
[]client.Object{newParentRefGatewayClass(), gw("gw", "apisix")},
+ parentRef: "gw",
+ wantGateways: 1,
+ wantUnresolved: false,
+ },
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ cli :=
fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.objects...).Build()
+
+ got, unresolved, err :=
ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route,
+ []gatewayv1.ParentReference{{Name:
gatewayv1.ObjectName(tc.parentRef)}})
+ require.NoError(t, err)
+ assert.Len(t, got, tc.wantGateways)
+ assert.Equal(t, tc.wantUnresolved, unresolved)
+ })
+ }
+}
diff --git a/internal/provider/apisix/provider.go
b/internal/provider/apisix/provider.go
index 86d7f532..039e3d75 100644
--- a/internal/provider/apisix/provider.go
+++ b/internal/provider/apisix/provider.go
@@ -228,13 +228,19 @@ func (d *apisixProvider) Delete(ctx context.Context, obj
client.Object) error {
Labels: labels,
})
}
- defer d.syncNotify()
- return d.client.DeleteConfig(ctx, adcclient.Task{
+ delta, err := d.client.DeleteConfig(ctx, adcclient.Task{
Key: nnk,
Name: nnk.String(),
Labels: labels,
ResourceTypes: resourceTypes,
})
+ // Syncing pushes the whole store to every data plane. Objects this
controller
+ // never configured delete nothing, and reconciles for them are
frequent, so
+ // notify only when the store actually changed.
+ if len(delta.Deleted) > 0 {
+ d.syncNotify()
+ }
+ return err
}
func (d *apisixProvider) buildConfig(tctx *provider.TranslateContext, nnk
types.NamespacedNameKind) (map[types.NamespacedNameKind]adctypes.Config, error)
{
diff --git a/internal/provider/apisix/provider_test.go
b/internal/provider/apisix/provider_test.go
new file mode 100644
index 00000000..e3be9b13
--- /dev/null
+++ b/internal/provider/apisix/provider_test.go
@@ -0,0 +1,67 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package apisix
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ "github.com/go-logr/logr"
+ "github.com/stretchr/testify/require"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ adctypes "github.com/apache/apisix-ingress-controller/api/adc"
+ adcclient
"github.com/apache/apisix-ingress-controller/internal/adc/client"
+ "github.com/apache/apisix-ingress-controller/internal/types"
+ "github.com/apache/apisix-ingress-controller/internal/utils"
+)
+
+// TestDeleteNotifiesSyncOnlyWhenConfigWasRemoved covers the cost side of route
+// ownership: a sync pushes the whole store to every data plane, and reconciles
+// for routes this controller never configured are frequent (any EndpointSlice
+// event on a shared backend enqueues them), so those must not notify.
+func TestDeleteNotifiesSyncOnlyWhenConfigWasRemoved(t *testing.T) {
+ cli, err := adcclient.New(logr.Discard(), ProviderTypeAPISIX,
time.Second)
+ require.NoError(t, err)
+
+ d := &apisixProvider{
+ client: cli,
+ syncCh: make(chan struct{}, 1),
+ log: logr.Discard(),
+ }
+
+ route := &gatewayv1.HTTPRoute{
+ TypeMeta: metav1.TypeMeta{
+ Kind: "HTTPRoute",
+ APIVersion: gatewayv1.GroupVersion.String(),
+ },
+ ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name:
"route"},
+ }
+
+ require.NoError(t, d.Delete(context.Background(), route))
+ require.Empty(t, d.syncCh, "a route this controller never configured
must not trigger a sync")
+
+ cli.ConfigManager.Update(utils.NamespacedNameKind(route),
map[types.NamespacedNameKind]adctypes.Config{
+ {Namespace: "default", Name: "proxy", Kind: "GatewayProxy"}:
{Name: "proxy"},
+ })
+
+ require.NoError(t, d.Delete(context.Background(), route))
+ require.Len(t, d.syncCh, 1, "removing configuration this controller
pushed must trigger a sync")
+}
diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go
index d21760b5..f8b70b8f 100644
--- a/test/e2e/gatewayapi/httproute.go
+++ b/test/e2e/gatewayapi/httproute.go
@@ -251,6 +251,82 @@ spec:
name: additional-proxy-config
`
+ // GatewayClass owned by a different controller, plus a Gateway
using it.
+ // Moving a route onto this Gateway takes it out of the scope
of the
+ // controller under test without deleting the route itself.
+ var foreignGatewayClassYaml = `
+apiVersion: gateway.networking.k8s.io/v1
+kind: GatewayClass
+metadata:
+ name: %s
+spec:
+ controllerName: "apisix.apache.org/not-exist"
+`
+
+ var foreignGateway = `
+apiVersion: gateway.networking.k8s.io/v1
+kind: Gateway
+metadata:
+ name: foreign-gateway
+spec:
+ gatewayClassName: %s
+ listeners:
+ - name: http-foreign
+ protocol: HTTP
+ port: 80
+ allowedRoutes:
+ namespaces:
+ from: All
+`
+
+ // HTTPRoute with a single parent, whose name is filled in by
the test.
+ var singleParentHTTPRoute = `
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+ name: moving-route
+spec:
+ parentRefs:
+ - name: %s
+ namespace: %s
+ hostnames:
+ - httpbin-additional.example
+ rules:
+ - matches:
+ - path:
+ type: Exact
+ value: /get
+ backendRefs:
+ - name: httpbin-service-e2e-test
+ port: 80
+`
+
+ // The same route with an extra match, so re-applying it bumps
the
+ // generation and forces a reconcile while the GatewayClass is
missing.
+ var singleParentHTTPRouteExtraMatch = `
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+ name: moving-route
+spec:
+ parentRefs:
+ - name: %s
+ namespace: %s
+ hostnames:
+ - httpbin-additional.example
+ rules:
+ - matches:
+ - path:
+ type: Exact
+ value: /get
+ - path:
+ type: Exact
+ value: /headers
+ backendRefs:
+ - name: httpbin-service-e2e-test
+ port: 80
+`
+
// HTTPRoute that references both gateways
var multiGatewayHTTPRoute = `
apiVersion: gateway.networking.k8s.io/v1
@@ -372,6 +448,100 @@ spec:
Check:
scaffold.WithExpectedStatus(http.StatusNotFound),
})
})
+
+ It("HTTPRoute should stop being served after moving to another
controller's Gateway", func() {
+ By("Create HTTPRoute on the additional gateway")
+ s.ResourceApplied("HTTPRoute", "moving-route",
+ fmt.Sprintf(singleParentHTTPRoute,
"additional-gateway", additionalSvc.Namespace), 1)
+
+ client, err :=
s.NewAPISIXClientForGateway(additionalGatewayGroupID)
+ Expect(err).NotTo(HaveOccurred(), "creating client for
additional gateway")
+
+ By("HTTPRoute should be accessible through the
additional gateway")
+ s.RequestAssert(&scaffold.RequestAssert{
+ Client: client,
+ Method: "GET",
+ Path: "/get",
+ Host: "httpbin-additional.example",
+ Check:
scaffold.WithExpectedStatus(http.StatusOK),
+ Timeout: time.Second * 30,
+ Interval: time.Second * 2,
+ })
+
+ By("Create a Gateway owned by another controller")
+ foreignGatewayClassName :=
fmt.Sprintf("foreign-gatewayclass-%d", time.Now().Nanosecond())
+ err = s.CreateResourceFromStringWithNamespace(
+ fmt.Sprintf(foreignGatewayClassYaml,
foreignGatewayClassName), "")
+ Expect(err).NotTo(HaveOccurred(), "creating foreign
GatewayClass")
+
+ err = s.CreateResourceFromStringWithNamespace(
+ fmt.Sprintf(foreignGateway,
foreignGatewayClassName), additionalSvc.Namespace)
+ Expect(err).NotTo(HaveOccurred(), "creating foreign
Gateway")
+
+ By("Move the HTTPRoute's parentRefs to that Gateway")
+ err = s.CreateResourceFromString(
+ fmt.Sprintf(singleParentHTTPRoute,
"foreign-gateway", additionalSvc.Namespace))
+ Expect(err).NotTo(HaveOccurred(), "moving HTTPRoute
parentRefs")
+
+ By("HTTPRoute should no longer be accessible through
the additional gateway")
+ s.RequestAssert(&scaffold.RequestAssert{
+ Client: client,
+ Method: "GET",
+ Path: "/get",
+ Host: "httpbin-additional.example",
+ Check:
scaffold.WithExpectedStatus(http.StatusNotFound),
+ Timeout: time.Second * 30,
+ Interval: time.Second * 2,
+ })
+ })
+
+ It("HTTPRoute should keep being served when its GatewayClass
disappears", func() {
+ By("Create HTTPRoute on the additional gateway")
+ s.ResourceApplied("HTTPRoute", "moving-route",
+ fmt.Sprintf(singleParentHTTPRoute,
"additional-gateway", additionalSvc.Namespace), 1)
+
+ client, err :=
s.NewAPISIXClientForGateway(additionalGatewayGroupID)
+ Expect(err).NotTo(HaveOccurred(), "creating client for
additional gateway")
+
+ By("HTTPRoute should be accessible through the
additional gateway")
+ s.RequestAssert(&scaffold.RequestAssert{
+ Client: client,
+ Method: "GET",
+ Path: "/get",
+ Host: "httpbin-additional.example",
+ Check:
scaffold.WithExpectedStatus(http.StatusOK),
+ Timeout: time.Second * 30,
+ Interval: time.Second * 2,
+ })
+
+ By("Delete the GatewayClass the additional Gateway
belongs to")
+ // gc-protection keeps the GatewayClass alive for as
long as a Gateway
+ // references it, so a plain delete only marks it
Terminating and the
+ // lookup still resolves. Dropping the finalizer
produces the state a
+ // CRD upgrade or a restore leaves behind: class gone,
Gateway alive.
+ _, err = s.RunKubectlAndGetOutput("delete",
"gatewayclass", additionalGatewayClassName, "--wait=false")
+ Expect(err).NotTo(HaveOccurred(), "deleting additional
GatewayClass")
+ _, err = s.RunKubectlAndGetOutput("patch",
"gatewayclass", additionalGatewayClassName,
+ "--type=merge", "-p",
`{"metadata":{"finalizers":[]}}`)
+ Expect(err).NotTo(HaveOccurred(), "removing the
GatewayClass finalizer")
+
+ s.RetryAssertion(func() error {
+ _, err := s.RunKubectlAndGetOutput("get",
"gatewayclass", additionalGatewayClassName)
+ return err
+ }).Should(HaveOccurred(), "GatewayClass should be gone")
+
+ By("Update the HTTPRoute so it reconciles while the
GatewayClass is gone")
+ err = s.CreateResourceFromString(
+ fmt.Sprintf(singleParentHTTPRouteExtraMatch,
"additional-gateway", additionalSvc.Namespace))
+ Expect(err).NotTo(HaveOccurred(), "updating HTTPRoute")
+
+ By("HTTPRoute should still be served: ownership is
unknown, not disproven")
+ request := func() int {
+ return
client.GET("/get").WithHost("httpbin-additional.example").Expect().Raw().StatusCode
+ }
+ Consistently(request).WithTimeout(time.Second *
30).ProbeEvery(time.Second * 2).
+ Should(Equal(http.StatusOK))
+ })
})
Context("HTTPRoute Base", func() {
