Skip to content

Commit 0a7b040

Browse files
AlinsRanAlinsRan
authored
fix: residual data issue when updating ingressClassName (#2543)
1 parent f172410 commit 0a7b040

File tree

16 files changed

+207
-162
lines changed

16 files changed

+207
-162
lines changed

‎internal/controller/apisixconsumer_controller.go‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func (r *ApisixConsumerReconciler) Reconcile(ctx context.Context, req ctrl.Reque
8383
ingressClass *networkingv1.IngressClass
8484
err error
8585
)
86-
if ingressClass, err = GetIngressClass(tctx, r.Client, r.Log, ac.Spec.IngressClassName); err != nil {
86+
if ingressClass, err = FindMatchingIngressClass(tctx, r.Client, r.Log, ac); err != nil {
8787
r.Log.V(1).Info("no matching IngressClass available",
8888
"ingressClassName", ac.Spec.IngressClassName,
8989
"error", err.Error())
@@ -113,7 +113,7 @@ func (r *ApisixConsumerReconciler) SetupWithManager(mgr ctrl.Manager) error {
113113
return ctrl.NewControllerManagedBy(mgr).
114114
For(&apiv2.ApisixConsumer{},
115115
builder.WithPredicates(
116-
predicate.NewPredicateFuncs(r.checkIngressClass),
116+
MatchesIngressClassPredicate(r.Client, r.Log),
117117
)).
118118
WithEventFilter(
119119
predicate.Or(
@@ -139,15 +139,6 @@ func (r *ApisixConsumerReconciler) SetupWithManager(mgr ctrl.Manager) error {
139139
Complete(r)
140140
}
141141

142-
func (r *ApisixConsumerReconciler) checkIngressClass(obj client.Object) bool {
143-
ac, ok := obj.(*apiv2.ApisixConsumer)
144-
if !ok {
145-
return false
146-
}
147-
148-
return matchesIngressClass(r.Client, r.Log, ac.Spec.IngressClassName)
149-
}
150-
151142
func (r *ApisixConsumerReconciler) listApisixConsumerForGatewayProxy(ctx context.Context, obj client.Object) []reconcile.Request {
152143
return listIngressClassRequestsForGatewayProxy(ctx, r.Client, obj, r.Log, r.listApisixConsumerForIngressClass)
153144
}

‎internal/controller/apisixglobalrule_controller.go‎

Lines changed: 6 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ import (
3535

3636
"github.com/apache/apisix-ingress-controller/api/v1alpha1"
3737
apiv2 "github.com/apache/apisix-ingress-controller/api/v2"
38-
"github.com/apache/apisix-ingress-controller/internal/controller/config"
39-
"github.com/apache/apisix-ingress-controller/internal/controller/indexer"
4038
"github.com/apache/apisix-ingress-controller/internal/controller/status"
4139
"github.com/apache/apisix-ingress-controller/internal/manager/readiness"
4240
"github.com/apache/apisix-ingress-controller/internal/provider"
@@ -84,11 +82,15 @@ func (r *ApisixGlobalRuleReconciler) Reconcile(ctx context.Context, req ctrl.Req
8482
tctx := provider.NewDefaultTranslateContext(ctx)
8583

8684
// get the ingress class
87-
ingressClass, err := GetIngressClass(tctx, r.Client, r.Log, globalRule.Spec.IngressClassName)
85+
ingressClass, err := FindMatchingIngressClass(tctx, r.Client, r.Log, &globalRule)
8886
if err != nil {
8987
r.Log.V(1).Info("no matching IngressClass available",
9088
"ingressClassName", globalRule.Spec.IngressClassName,
9189
"error", err.Error())
90+
if err := r.Provider.Delete(ctx, &globalRule); err != nil {
91+
r.Log.Error(err, "failed to delete global rule from provider")
92+
return ctrl.Result{}, err
93+
}
9294
return ctrl.Result{}, nil
9395
}
9496

@@ -131,7 +133,7 @@ func (r *ApisixGlobalRuleReconciler) SetupWithManager(mgr ctrl.Manager) error {
131133
return ctrl.NewControllerManagedBy(mgr).
132134
For(&apiv2.ApisixGlobalRule{},
133135
builder.WithPredicates(
134-
predicate.NewPredicateFuncs(r.checkIngressClass),
136+
MatchesIngressClassPredicate(r.Client, r.Log),
135137
),
136138
).
137139
WithEventFilter(
@@ -154,47 +156,6 @@ func (r *ApisixGlobalRuleReconciler) SetupWithManager(mgr ctrl.Manager) error {
154156
Complete(r)
155157
}
156158

157-
// checkIngressClass checks if the ApisixGlobalRule uses the ingress class that we control
158-
func (r *ApisixGlobalRuleReconciler) checkIngressClass(obj client.Object) bool {
159-
globalRule, ok := obj.(*apiv2.ApisixGlobalRule)
160-
if !ok {
161-
return false
162-
}
163-
164-
return r.matchesIngressClass(globalRule.Spec.IngressClassName)
165-
}
166-
167-
// matchesIngressClass checks if the given ingress class name matches our controlled classes
168-
func (r *ApisixGlobalRuleReconciler) matchesIngressClass(ingressClassName string) bool {
169-
if ingressClassName == "" {
170-
// Check for default ingress class
171-
ingressClassList := &networkingv1.IngressClassList{}
172-
if err := r.List(context.Background(), ingressClassList, client.MatchingFields{
173-
indexer.IngressClass: config.GetControllerName(),
174-
}); err != nil {
175-
r.Log.Error(err, "failed to list ingress classes")
176-
return false
177-
}
178-
179-
// Find the ingress class that is marked as default
180-
for _, ic := range ingressClassList.Items {
181-
if IsDefaultIngressClass(&ic) && matchesController(ic.Spec.Controller) {
182-
return true
183-
}
184-
}
185-
return false
186-
}
187-
188-
// Check if the specified ingress class is controlled by us
189-
var ingressClass networkingv1.IngressClass
190-
if err := r.Get(context.Background(), client.ObjectKey{Name: ingressClassName}, &ingressClass); err != nil {
191-
r.Log.Error(err, "failed to get ingress class", "ingressClass", ingressClassName)
192-
return false
193-
}
194-
195-
return matchesController(ingressClass.Spec.Controller)
196-
}
197-
198159
// listGlobalRulesForIngressClass list all global rules that use a specific ingress class
199160
func (r *ApisixGlobalRuleReconciler) listGlobalRulesForIngressClass(ctx context.Context, obj client.Object) []reconcile.Request {
200161
ingressClass, ok := obj.(*networkingv1.IngressClass)

‎internal/controller/apisixpluginconfig_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (r *ApisixPluginConfigReconciler) Reconcile(ctx context.Context, req ctrl.R
5858

5959
tctx := provider.NewDefaultTranslateContext(ctx)
6060

61-
_, err := GetIngressClass(tctx, r.Client, r.Log, pc.Spec.IngressClassName)
61+
_, err := FindMatchingIngressClass(tctx, r.Client, r.Log, &pc)
6262
if err != nil {
6363
r.Log.V(1).Info("no matching IngressClass available",
6464
"ingressClassName", pc.Spec.IngressClassName,

‎internal/controller/apisixroute_controller.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,11 @@ type ApisixRouteReconciler struct {
6464
// SetupWithManager sets up the controller with the Manager.
6565
func (r *ApisixRouteReconciler) SetupWithManager(mgr ctrl.Manager) error {
6666
return ctrl.NewControllerManagedBy(mgr).
67-
For(&apiv2.ApisixRoute{}).
67+
For(&apiv2.ApisixRoute{},
68+
builder.WithPredicates(
69+
MatchesIngressClassPredicate(r.Client, r.Log),
70+
),
71+
).
6872
WithEventFilter(
6973
predicate.Or(
7074
predicate.GenerationChangedPredicate{},
@@ -125,10 +129,14 @@ func (r *ApisixRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request)
125129
err error
126130
)
127131

128-
if ic, err = GetIngressClass(tctx, r.Client, r.Log, ar.Spec.IngressClassName); err != nil {
132+
if ic, err = FindMatchingIngressClass(tctx, r.Client, r.Log, &ar); err != nil {
129133
r.Log.V(1).Info("no matching IngressClass available",
130134
"ingressClassName", ar.Spec.IngressClassName,
131135
"error", err.Error())
136+
if err := r.Provider.Delete(ctx, &ar); err != nil {
137+
r.Log.Error(err, "failed to delete apisixroute", "apisixroute", ar)
138+
return ctrl.Result{}, err
139+
}
132140
return ctrl.Result{}, nil
133141
}
134142
defer func() { r.updateStatus(&ar, err) }()

‎internal/controller/apisixtls_controller.go‎

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636

3737
"github.com/apache/apisix-ingress-controller/api/v1alpha1"
3838
apiv2 "github.com/apache/apisix-ingress-controller/api/v2"
39-
"github.com/apache/apisix-ingress-controller/internal/controller/config"
4039
"github.com/apache/apisix-ingress-controller/internal/controller/indexer"
4140
"github.com/apache/apisix-ingress-controller/internal/controller/status"
4241
"github.com/apache/apisix-ingress-controller/internal/manager/readiness"
@@ -59,7 +58,7 @@ func (r *ApisixTlsReconciler) SetupWithManager(mgr ctrl.Manager) error {
5958
return ctrl.NewControllerManagedBy(mgr).
6059
For(&apiv2.ApisixTls{},
6160
builder.WithPredicates(
62-
predicate.NewPredicateFuncs(r.checkIngressClass),
61+
MatchesIngressClassPredicate(r.Client, r.Log),
6362
),
6463
).
6564
WithEventFilter(
@@ -115,7 +114,7 @@ func (r *ApisixTlsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
115114
tctx := provider.NewDefaultTranslateContext(ctx)
116115

117116
// get the ingress class
118-
ingressClass, err := GetIngressClass(tctx, r.Client, r.Log, tls.Spec.IngressClassName)
117+
ingressClass, err := FindMatchingIngressClass(tctx, r.Client, r.Log, &tls)
119118
if err != nil {
120119
r.Log.V(1).Info("no matching IngressClass available, skip processing",
121120
"ingressClassName", tls.Spec.IngressClassName,
@@ -227,47 +226,6 @@ func (r *ApisixTlsReconciler) updateStatus(tls *apiv2.ApisixTls, condition metav
227226
})
228227
}
229228

230-
// checkIngressClass checks if the ApisixTls uses the ingress class that we control
231-
func (r *ApisixTlsReconciler) checkIngressClass(obj client.Object) bool {
232-
tls, ok := obj.(*apiv2.ApisixTls)
233-
if !ok {
234-
return false
235-
}
236-
237-
return r.matchesIngressClass(tls.Spec.IngressClassName)
238-
}
239-
240-
// matchesIngressClass checks if the given ingress class name matches our controlled classes
241-
func (r *ApisixTlsReconciler) matchesIngressClass(ingressClassName string) bool {
242-
if ingressClassName == "" {
243-
// Check for default ingress class
244-
ingressClassList := &networkingv1.IngressClassList{}
245-
if err := r.List(context.Background(), ingressClassList, client.MatchingFields{
246-
indexer.IngressClass: config.GetControllerName(),
247-
}); err != nil {
248-
r.Log.Error(err, "failed to list ingress classes")
249-
return false
250-
}
251-
252-
// Find the ingress class that is marked as default
253-
for _, ic := range ingressClassList.Items {
254-
if IsDefaultIngressClass(&ic) && matchesController(ic.Spec.Controller) {
255-
return true
256-
}
257-
}
258-
return false
259-
}
260-
261-
// Check if the specified ingress class is controlled by us
262-
var ingressClass networkingv1.IngressClass
263-
if err := r.Get(context.Background(), client.ObjectKey{Name: ingressClassName}, &ingressClass); err != nil {
264-
r.Log.Error(err, "failed to get ingress class", "ingressClass", ingressClassName)
265-
return false
266-
}
267-
268-
return matchesController(ingressClass.Spec.Controller)
269-
}
270-
271229
func (r *ApisixTlsReconciler) listApisixTlsForSecret(ctx context.Context, obj client.Object) []reconcile.Request {
272230
secret, ok := obj.(*corev1.Secret)
273231
if !ok {

‎internal/controller/apisixupstream_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (r *ApisixUpstreamReconciler) Reconcile(ctx context.Context, req ctrl.Reque
5757

5858
tctx := provider.NewDefaultTranslateContext(ctx)
5959

60-
_, err := GetIngressClass(tctx, r.Client, r.Log, au.Spec.IngressClassName)
60+
_, err := FindMatchingIngressClass(tctx, r.Client, r.Log, &au)
6161
if err != nil {
6262
r.Log.V(1).Info("no matching IngressClass available, skip processing",
6363
"ingressClassName", au.Spec.IngressClassName,

‎internal/controller/httproutepolicy.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func (r *IngressReconciler) updateHTTPRoutePolicyStatusOnDeleting(ctx context.Co
198198
if err := r.Get(ctx, namespacedName, &ingress); err != nil {
199199
continue
200200
}
201-
ingressClass, err := r.getIngressClass(ctx, &ingress)
201+
ingressClass, err := FindMatchingIngressClass(ctx, r.Client, r.Log, &ingress)
202202
if err != nil {
203203
continue
204204
}

‎internal/controller/ingress_controller.go‎

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (r *IngressReconciler) SetupWithManager(mgr ctrl.Manager) error {
7070
return ctrl.NewControllerManagedBy(mgr).
7171
For(&networkingv1.Ingress{},
7272
builder.WithPredicates(
73-
predicate.NewPredicateFuncs(r.checkIngressClass),
73+
MatchesIngressClassPredicate(r.Client, r.Log),
7474
),
7575
).
7676
WithEventFilter(
@@ -151,10 +151,13 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
151151
// create a translate context
152152
tctx := provider.NewDefaultTranslateContext(ctx)
153153

154-
ingressClass, err := r.getIngressClass(ctx, ingress)
154+
ingressClass, err := FindMatchingIngressClass(tctx, r.Client, r.Log, ingress)
155155
if err != nil {
156-
r.Log.Error(err, "failed to get IngressClass")
157-
return ctrl.Result{}, err
156+
if err := r.Provider.Delete(ctx, ingress); err != nil {
157+
r.Log.Error(err, "failed to delete ingress resources", "ingress", ingress.Name)
158+
return ctrl.Result{}, nil
159+
}
160+
return ctrl.Result{}, nil
158161
}
159162

160163
tctx.RouteParentRefs = append(tctx.RouteParentRefs, gatewayv1.ParentReference{
@@ -207,22 +210,6 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
207210
return ctrl.Result{}, nil
208211
}
209212

210-
// getIngressClass get the ingress class for the ingress
211-
func (r *IngressReconciler) getIngressClass(ctx context.Context, obj client.Object) (*networkingv1.IngressClass, error) {
212-
ingress := obj.(*networkingv1.Ingress)
213-
var ingressClassName string
214-
if ingress.Spec.IngressClassName != nil {
215-
ingressClassName = *ingress.Spec.IngressClassName
216-
}
217-
return GetIngressClass(ctx, r.Client, r.Log, ingressClassName)
218-
}
219-
220-
// checkIngressClass check if the ingress uses the ingress class that we control
221-
func (r *IngressReconciler) checkIngressClass(obj client.Object) bool {
222-
_, err := r.getIngressClass(context.Background(), obj)
223-
return err == nil
224-
}
225-
226213
// matchesIngressController check if the ingress class is controlled by us
227214
func (r *IngressReconciler) matchesIngressController(obj client.Object) bool {
228215
ingressClass, ok := obj.(*networkingv1.IngressClass)
@@ -307,7 +294,7 @@ func (r *IngressReconciler) listIngressesByService(ctx context.Context, obj clie
307294

308295
requests := make([]reconcile.Request, 0, len(ingressList.Items))
309296
for _, ingress := range ingressList.Items {
310-
if r.checkIngressClass(&ingress) {
297+
if MatchesIngressClass(r.Client, r.Log, &ingress) {
311298
requests = append(requests, reconcile.Request{
312299
NamespacedName: client.ObjectKey{
313300
Namespace: ingress.Namespace,
@@ -340,7 +327,7 @@ func (r *IngressReconciler) listIngressesBySecret(ctx context.Context, obj clien
340327

341328
requests := make([]reconcile.Request, 0, len(ingressList.Items))
342329
for _, ingress := range ingressList.Items {
343-
if r.checkIngressClass(&ingress) {
330+
if MatchesIngressClass(r.Client, r.Log, &ingress) {
344331
requests = append(requests, reconcile.Request{
345332
NamespacedName: client.ObjectKey{
346333
Namespace: ingress.Namespace,

0 commit comments

Comments
 (0)