Skip to content

Commit 6d1cdad

Browse files
hdurand0710oktalz
hdurand0710
authored and
oktalz
committed
BUG/MEDIUM: enqueueHTTPRouteForService did not properly enqueue an HTTPRoute on Service update or delete
This bug was introduced in commit CLEANUP/MEDIUM: use standard EnqueueFor mechanism The consequence was that the HTTPRoute was not re-enqueued when a referenced Service was modified or deleted. The Rule validity was not re computed, stayed Valid and then the Backend was not deleted. Note that after this MR only HTTPRoute are correctly handled.
1 parent 6a996c7 commit 6d1cdad

File tree

8 files changed

+183
-17
lines changed

8 files changed

+183
-17
lines changed

‎k8s/gate/enqueuefor.go‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -252,24 +252,22 @@ func enqueueHTTPRouteForService(ctrlclient client.Client, extractGVK utilsk8s.Ex
252252
}
253253

254254
for _, route := range routeList.Items {
255-
for _, parentRef := range route.Spec.ParentRefs {
256-
for _, rule := range route.Spec.Rules {
257-
for _, backendRef := range rule.BackendRefs {
258-
// We only accept v1.Service
259-
if !utilsk8s.IsBackendRefGroupKindSupported(backendRef.BackendObjectReference, extractGVK) {
260-
continue
261-
}
262-
serviceNsName := utils.GetNamespacedName(
263-
string(parentRef.Name),
264-
string(utils.PointerDefaultValueIfNil(parentRef.Namespace)),
265-
route.GetNamespace())
255+
for _, rule := range route.Spec.Rules {
256+
for _, backendRef := range rule.BackendRefs {
257+
// We only accept v1.Service
258+
if !utilsk8s.IsBackendRefGroupKindSupported(backendRef.BackendObjectReference, extractGVK) {
259+
continue
260+
}
261+
serviceNsName := utils.GetNamespacedName(
262+
string(backendRef.Name),
263+
string(utils.PointerDefaultValueIfNil(backendRef.Namespace)),
264+
route.GetNamespace())
266265

267-
if serviceNsName.Name == o.GetName() && serviceNsName.Namespace == o.GetNamespace() {
268-
requests = append(requests, reconcile.Request{NamespacedName: types.NamespacedName{
269-
Namespace: route.GetNamespace(),
270-
Name: route.GetName(),
271-
}})
272-
}
266+
if serviceNsName.Name == o.GetName() && serviceNsName.Namespace == o.GetNamespace() {
267+
requests = append(requests, reconcile.Request{NamespacedName: types.NamespacedName{
268+
Namespace: route.GetNamespace(),
269+
Name: route.GetName(),
270+
}})
273271
}
274272
}
275273
}

‎test/integration/httproute/httproute_backends_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import (
1919
"path"
2020

2121
"github.com/haproxytech/haproxy-unified-gateway/test/integration/utils"
22+
v1 "k8s.io/api/core/v1"
23+
"sigs.k8s.io/controller-runtime/pkg/client"
2224
)
2325

2426
func (s *HTTPRouteTestSuite) Test_HTTPRoute_Backend_1_route() {
@@ -114,3 +116,46 @@ func (s *HTTPRouteTestSuite) Test_HTTPRoute_Backend_1_route_dynamic_delete_1_bac
114116
s.ExpectBackends(s.Test().Ctx, backendsExpectationsPath, expectedBackends)
115117
s.ExpectBackendsDoNotExist(s.Test().Ctx, "link1_e2e-tests-httproute_http-echo-2_80__")
116118
}
119+
120+
func (s *HTTPRouteTestSuite) Test_HTTPRoute_Backend_1_route_1_backend_dynamic_delete_service() {
121+
fixtureDirPath := utils.GetCRDFixturePath()
122+
fixtureDir := "backends"
123+
124+
fixturePath := path.Join(fixtureDirPath, fixtureDir, "1_route_1_backend")
125+
manifests := []string{"gateway.yaml", "gatewayclass.yaml", "http-echo-1.yaml", "route-1.yaml"}
126+
s.CreateFixtures(fixturePath, manifests)
127+
defer s.CleanupFixtures(fixturePath, manifests)
128+
129+
// Expected Conditions
130+
expectationsPath := path.Join(fixturePath, "expectations")
131+
expectedCondPath := path.Join(expectationsPath, "conditions.yaml")
132+
expectedConditions := s.YamlToRouteConditions(expectedCondPath)
133+
134+
httpRouteName := "route-echo-1"
135+
s.expectConditionsUpdated(s.Test().Ctx, s.Test().Namespace, httpRouteName, expectedConditions)
136+
137+
// Check AttachedRoutes on Gateway status
138+
s.expectAttachedRoute(s.Test().Ctx, s.Test().Namespace, "gateway", "http", 1)
139+
140+
// haproxy.cfg Backends
141+
backendsExpectationsPath := path.Join(expectationsPath, "backends")
142+
expectedBackends := []string{"link1_e2e-tests-httproute_http-echo-1_80__"}
143+
s.ExpectBackends(s.Test().Ctx, backendsExpectationsPath, expectedBackends)
144+
145+
// Now remove the Service http-echo-1
146+
// Backend link1_e2e-tests-httproute_http-echo-1_80__ should be deleted
147+
s.deleteService("http-echo-1")
148+
s.ExpectBackendsDoNotExist(s.Test().Ctx, "link1_e2e-tests-httproute_http-echo-1_80__")
149+
s.CreateFixtures(fixturePath, []string{"http-echo-1.yaml"}) // Recreate the service in order to please the defer s.CleanupFixtures(fixturePath, manifests)
150+
}
151+
152+
func (s *HTTPRouteTestSuite) deleteService(name string) *v1.Service {
153+
var service v1.Service
154+
err := s.Test().Client.Get(s.Test().Ctx, client.ObjectKey{Name: name, Namespace: s.Test().Namespace}, &service)
155+
s.Require().NoError(err)
156+
157+
err = s.Test().Client.Delete(s.Test().Ctx, &service)
158+
s.Require().NoError(err)
159+
160+
return &service
161+
}

‎test/integration/httproute/manifests/backends/1_route_1_backend/expectations/backends/link1_e2e-tests-httproute_http-echo-1_80__.yaml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{
2+
"from": "haproxytech",
3+
"metadata": {
4+
"hug": {
5+
"HTTPRoute": {
6+
"e2e-tests-httproute/route-echo-1": {
7+
"Generation": 1,
8+
"LinkID": "link1"
9+
}
10+
}
11+
}
12+
},
13+
"mode": "http",
14+
"name": "link1_e2e-tests-httproute_http-echo-1_80__",
15+
"abortonclose": "disabled",
16+
"balance": {
17+
"algorithm": "roundrobin"
18+
},
19+
"default_server": {
20+
"check": "enabled"
21+
},
22+
"forwardfor": {
23+
"enabled": "enabled"
24+
},
25+
"server_timeout": 50000
26+
}

‎test/integration/httproute/manifests/backends/1_route_1_backend/expectations/conditions.yaml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
parents:
2+
- conditions:
3+
- message: Route Accepted
4+
reason: Accepted
5+
status: "True"
6+
type: Accepted
7+
- message: References resolved
8+
reason: ResolvedRefs
9+
status: "True"
10+
type: ResolvedRefs
11+
controllerName: gate.haproxy.org/hug
12+
parentRef:
13+
group: gateway.networking.k8s.io
14+
kind: Gateway
15+
name: gateway
16+
sectionName: http

‎test/integration/httproute/manifests/backends/1_route_1_backend/gateway.yaml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
apiVersion: gateway.networking.k8s.io/v1
2+
kind: Gateway
3+
metadata:
4+
name: gateway
5+
spec:
6+
gatewayClassName: haproxy
7+
listeners:
8+
- name: http
9+
port: 8080
10+
protocol: HTTP
11+
allowedRoutes:
12+
kinds:
13+
- group: gateway.networking.k8s.io
14+
kind: HTTPRoute
15+
hostname: "*.haproxy"

‎test/integration/httproute/manifests/backends/1_route_1_backend/gatewayclass.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
apiVersion: gateway.networking.k8s.io/v1
2+
kind: GatewayClass
3+
metadata:
4+
name: haproxy
5+
spec:
6+
controllerName: gate.haproxy.org/hug

‎test/integration/httproute/manifests/backends/1_route_1_backend/http-echo-1.yaml‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
kind: Deployment
2+
apiVersion: apps/v1
3+
metadata:
4+
name: http-echo-1
5+
spec:
6+
replicas: 1
7+
selector:
8+
matchLabels:
9+
app: http-echo-1
10+
template:
11+
metadata:
12+
labels:
13+
app: http-echo-1
14+
spec:
15+
containers:
16+
- name: http-echo
17+
image: "haproxytech/http-echo:latest"
18+
imagePullPolicy: Never
19+
ports:
20+
- name: http
21+
containerPort: 8888
22+
protocol: TCP
23+
- name: https
24+
containerPort: 8443
25+
protocol: TCP
26+
---
27+
kind: Service
28+
apiVersion: v1
29+
metadata:
30+
name: http-echo-1
31+
spec:
32+
ports:
33+
- name: http
34+
protocol: TCP
35+
port: 80
36+
targetPort: http
37+
- name: https
38+
protocol: TCP
39+
port: 443
40+
targetPort: https
41+
selector:
42+
app: http-echo-1

‎test/integration/httproute/manifests/backends/1_route_1_backend/route-1.yaml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
apiVersion: gateway.networking.k8s.io/v1
2+
kind: HTTPRoute
3+
metadata:
4+
name: route-echo-1
5+
spec:
6+
parentRefs:
7+
- name: gateway
8+
sectionName: http
9+
hostnames:
10+
- "example.haproxy"
11+
rules:
12+
- matches:
13+
- path:
14+
type: PathPrefix
15+
value: /route1
16+
backendRefs:
17+
- name: http-echo-1
18+
port: 80

0 commit comments

Comments
 (0)