Skip to content

Commit 76ff1df

Browse files
AlinsRanAlinsRan
authored
fix: status should not be recorded when ingressclass does not match (#234)
1 parent b87e04c commit 76ff1df

File tree

11 files changed

+93
-27
lines changed

11 files changed

+93
-27
lines changed

‎Makefile‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ kind-load-images: pull-infra-images kind-load-ingress-image
190190
@kind load docker-image hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-integrated:$(DASHBOARD_VERSION) --name $(KIND_NAME)
191191
@kind load docker-image kennethreitz/httpbin:latest --name $(KIND_NAME)
192192
@kind load docker-image jmalloc/echo-server:latest --name $(KIND_NAME)
193+
@kind load docker-image ghcr.io/api7/adc:dev --name $(KIND_NAME)
193194

194195
.PHONY: kind-load-gateway-image
195196
kind-load-gateway-image:
@@ -215,6 +216,7 @@ pull-infra-images:
215216
@docker pull hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-integrated:$(DASHBOARD_VERSION)
216217
@docker pull kennethreitz/httpbin:latest
217218
@docker pull jmalloc/echo-server:latest
219+
@docker pull ghcr.io/api7/adc:dev
218220

219221
##@ Build
220222

@@ -362,7 +364,7 @@ $(ENVTEST): $(LOCALBIN)
362364
.PHONY: golangci-lint
363365
golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary.
364366
$(GOLANGCI_LINT): $(LOCALBIN)
365-
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s $(GOLANGCI_LINT_VERSION)
367+
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(LOCALBIN) $(GOLANGCI_LINT_VERSION)
366368

367369
gofmt: ## Apply go fmt
368370
@gofmt -w -r 'interface{} -> any' .

‎internal/controller/apisixconsumer_controller.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,22 +89,22 @@ func (r *ApisixConsumerReconciler) Reconcile(ctx context.Context, req ctrl.Reque
8989
ingressClass *networkingv1.IngressClass
9090
err error
9191
)
92-
defer func() {
93-
r.updateStatus(ac, err)
94-
}()
9592

96-
ingressClass, err = GetIngressClass(tctx, r.Client, r.Log, ac.Spec.IngressClassName, r.ICGV.String())
97-
if err != nil {
98-
r.Log.Error(err, "failed to get IngressClass")
99-
return ctrl.Result{}, err
93+
if ingressClass, err = GetIngressClass(tctx, r.Client, r.Log, ac.Spec.IngressClassName, r.ICGV.String()); err != nil {
94+
r.Log.V(1).Info("no matching IngressClass available",
95+
"ingressClassName", ac.Spec.IngressClassName,
96+
"error", err.Error())
97+
return ctrl.Result{}, nil
10098
}
99+
defer func() { r.updateStatus(ac, err) }()
101100

102101
if err = ProcessIngressClassParameters(tctx, r.Client, r.Log, ac, ingressClass); err != nil {
103102
r.Log.Error(err, "failed to process IngressClass parameters", "ingressClass", ingressClass.Name)
104103
return ctrl.Result{}, err
105104
}
106105

107106
if err = r.processSpec(ctx, tctx, ac); err != nil {
107+
r.Log.Error(err, "failed to process ApisixConsumer spec", "object", ac)
108108
return ctrl.Result{}, err
109109
}
110110

‎internal/controller/apisixglobalrule_controller.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,18 @@ func (r *ApisixGlobalRuleReconciler) Reconcile(ctx context.Context, req ctrl.Req
8181
return ctrl.Result{}, err
8282
}
8383

84-
r.Log.Info("reconciling global rule", "globalrule", globalRule.Name)
84+
r.Log.V(1).Info("reconciling ApisixGlobalRule", "object", globalRule)
8585

8686
// create a translate context
8787
tctx := provider.NewDefaultTranslateContext(ctx)
8888

8989
// get the ingress class
9090
ingressClass, err := GetIngressClass(tctx, r.Client, r.Log, globalRule.Spec.IngressClassName, r.ICGV.String())
9191
if err != nil {
92-
r.Log.Error(err, "failed to get IngressClass")
93-
return ctrl.Result{}, err
92+
r.Log.V(1).Info("no matching IngressClass available",
93+
"ingressClassName", globalRule.Spec.IngressClassName,
94+
"error", err.Error())
95+
return ctrl.Result{}, nil
9496
}
9597

9698
// process IngressClass parameters if they reference GatewayProxy

‎internal/controller/apisixpluginconfig_controller.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030

3131
apiv2 "github.com/apache/apisix-ingress-controller/api/v2"
3232
"github.com/apache/apisix-ingress-controller/internal/controller/status"
33+
"github.com/apache/apisix-ingress-controller/internal/provider"
3334
"github.com/apache/apisix-ingress-controller/internal/utils"
3435
)
3536

@@ -57,6 +58,15 @@ func (r *ApisixPluginConfigReconciler) Reconcile(ctx context.Context, req ctrl.R
5758
if err := r.Get(ctx, req.NamespacedName, &pc); err != nil {
5859
return ctrl.Result{}, client.IgnoreNotFound(err)
5960
}
61+
tctx := provider.NewDefaultTranslateContext(ctx)
62+
63+
_, err := GetIngressClass(tctx, r.Client, r.Log, pc.Spec.IngressClassName, r.ICGV.String())
64+
if err != nil {
65+
r.Log.V(1).Info("no matching IngressClass available",
66+
"ingressClassName", pc.Spec.IngressClassName,
67+
"error", err.Error())
68+
return ctrl.Result{}, nil
69+
}
6070

6171
// Only update status
6272
r.updateStatus(&pc, nil)

‎internal/controller/apisixroute_controller.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,15 @@ func (r *ApisixRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request)
150150
ic *networkingv1.IngressClass
151151
err error
152152
)
153-
defer func() {
154-
r.updateStatus(&ar, err)
155-
}()
156153

157154
if ic, err = GetIngressClass(tctx, r.Client, r.Log, ar.Spec.IngressClassName, r.ICGV.String()); err != nil {
158-
return ctrl.Result{}, err
155+
r.Log.V(1).Info("no matching IngressClass available",
156+
"ingressClassName", ar.Spec.IngressClassName,
157+
"error", err.Error())
158+
return ctrl.Result{}, nil
159159
}
160+
defer func() { r.updateStatus(&ar, err) }()
161+
160162
if err = ProcessIngressClassParameters(tctx, r.Client, r.Log, &ar, ic); err != nil {
161163
return ctrl.Result{}, err
162164
}

‎internal/controller/apisixtls_controller.go‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,16 +128,10 @@ func (r *ApisixTlsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
128128
// get the ingress class
129129
ingressClass, err := GetIngressClass(tctx, r.Client, r.Log, tls.Spec.IngressClassName, r.ICGV.String())
130130
if err != nil {
131-
r.Log.Error(err, "failed to get IngressClass")
132-
r.updateStatus(&tls, metav1.Condition{
133-
Type: string(apiv2.ConditionTypeAccepted),
134-
Status: metav1.ConditionFalse,
135-
ObservedGeneration: tls.Generation,
136-
LastTransitionTime: metav1.Now(),
137-
Reason: string(apiv2.ConditionReasonInvalidSpec),
138-
Message: err.Error(),
139-
})
140-
return ctrl.Result{}, err
131+
r.Log.V(1).Info("no matching IngressClass available, skip processing",
132+
"ingressClassName", tls.Spec.IngressClassName,
133+
"error", err.Error())
134+
return ctrl.Result{}, nil
141135
}
142136

143137
// process IngressClass parameters if they reference GatewayProxy

‎internal/controller/apisixupstream_controller.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929

3030
apiv2 "github.com/apache/apisix-ingress-controller/api/v2"
3131
"github.com/apache/apisix-ingress-controller/internal/controller/status"
32+
"github.com/apache/apisix-ingress-controller/internal/provider"
3233
"github.com/apache/apisix-ingress-controller/internal/utils"
3334
)
3435

@@ -57,6 +58,16 @@ func (r *ApisixUpstreamReconciler) Reconcile(ctx context.Context, req ctrl.Reque
5758
return ctrl.Result{}, client.IgnoreNotFound(err)
5859
}
5960

61+
tctx := provider.NewDefaultTranslateContext(ctx)
62+
63+
_, err := GetIngressClass(tctx, r.Client, r.Log, au.Spec.IngressClassName, r.ICGV.String())
64+
if err != nil {
65+
r.Log.V(1).Info("no matching IngressClass available, skip processing",
66+
"ingressClassName", au.Spec.IngressClassName,
67+
"error", err.Error())
68+
return ctrl.Result{}, nil
69+
}
70+
6071
// Only update status
6172
r.updateStatus(&au, nil)
6273
return ctrl.Result{}, nil

‎test/e2e/crds/v2/status.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,25 @@ spec:
110110
- name: non-existent-plugin
111111
enable: true
112112
`
113+
const arWithInvalidIngressClass = `
114+
apiVersion: apisix.apache.org/v2
115+
kind: ApisixRoute
116+
metadata:
117+
name: ar-with-invalid-ingressclass
118+
spec:
119+
ingressClassName: ar-with-invalid-ingressclass
120+
http:
121+
- name: rule0
122+
match:
123+
hosts:
124+
- httpbin
125+
paths:
126+
- /*
127+
backends:
128+
- serviceName: httpbin-service-e2e-test
129+
servicePort: 80
130+
`
131+
113132
It("unknown plugin", func() {
114133
if os.Getenv("PROVIDER_TYPE") == "apisix-standalone" {
115134
Skip("apisix standalone does not validate unknown plugins")
@@ -258,5 +277,22 @@ spec:
258277
Expect(route2.Status.Conditions[0].LastTransitionTime).To(Equal(route.Status.Conditions[0].LastTransitionTime),
259278
"should not update the same status condition again")
260279
})
280+
281+
It("IngressClass not found", func() {
282+
err := s.CreateResourceFromString(arWithInvalidIngressClass)
283+
Expect(err).NotTo(HaveOccurred(), "creating ApisixRoute with invalid IngressClass")
284+
285+
for range 10 {
286+
output, err := s.GetOutputFromString("ar", "ar-with-invalid-ingressclass", "-o", "yaml")
287+
Expect(err).NotTo(HaveOccurred(), "getting ApisixRoute output")
288+
Expect(output).ShouldNot(
289+
Or(
290+
ContainSubstring(`status: "True"`),
291+
ContainSubstring(`status: "False"`),
292+
),
293+
)
294+
time.Sleep(1 * time.Second)
295+
}
296+
})
261297
})
262298
})

‎test/e2e/ingress/ingressclass.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package ingress

‎test/e2e/scaffold/api7_deployer.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,14 @@ func (s *API7Deployer) BeforeEach() {
9292
e := utils.ParallelExecutor{}
9393

9494
e.Add(func() {
95+
defer GinkgoRecover()
9596
s.DeployDataplane(DeployDataplaneOptions{})
9697
s.DeployIngress()
9798
})
98-
e.Add(s.DeployTestService)
99+
e.Add(func() {
100+
defer GinkgoRecover()
101+
s.DeployTestService()
102+
})
99103
e.Wait()
100104
}
101105

0 commit comments

Comments
 (0)