Skip to content

Commit e61097d

Browse files
vgramerGopher Bot
vgramer
authored and
Gopher Bot
committed
BUG/MINOR: HTTP Rule: avoid unnecessary reload when only maps are changed.
The haproxy configuration contains metadata (ie comments) about which k8s object "generated" this configuration. The metadata contains the k8s generation version, so when we edit the spec of an HTTPRule the generation version change and configuration is updated. However not all changes on the HTTPRule spec should trigger a reload. For example if we update the PathPrefix only maps should be updated and this action can be do on the runtime. In this case we just write the updated HAProxy configuration file with the updated comment but we do not trigger a reload. Signed-off-by: Vincent Gramer <vgramer@haproxy.com>
1 parent 3520307 commit e61097d

29 files changed

Lines changed: 530 additions & 15 deletions

‎hug/haproxy/api/backend.go‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"context"
1818
"log/slog"
1919

20-
parser "github.com/haproxytech/client-native/v6/config-parser"
2120
"github.com/haproxytech/client-native/v6/models"
2221
"github.com/haproxytech/haproxy-unified-gateway/hug/reload"
2322
"github.com/haproxytech/haproxy-unified-gateway/k8s/gate/logging"
@@ -90,15 +89,19 @@ func (c *clientNative) BackendEdit(backend models.Backend) error {
9089
return err
9190
}
9291

93-
// Check if only Servers were updated
94-
onlyServersUpdated := false
95-
if cmp.Equal(previousBackend, backend, cmpopts.IgnoreFields(models.Backend{}, "Servers")) {
96-
c.logger.LogAttrs(
97-
context.Background(), slog.LevelInfo, "Only Servers are updated",
98-
slog.String("backend", backend.Name),
99-
)
100-
onlyServersUpdated = true
101-
}
92+
serverChanged := !cmp.Equal(previousBackend.Servers, backend.Servers, cmpopts.EquateEmpty())
93+
// ignoring metadata because it contains the generation of the route which changes every time the route's spec is updated.
94+
// but some changes on the routes (ie PathPrefix) only affect maps.
95+
backendChanged := !cmp.Equal(previousBackend, backend, cmpopts.IgnoreFields(models.Backend{}, "Servers", "BackendBase.Metadata"))
96+
onlyServersUpdated := serverChanged && !backendChanged
97+
98+
c.logger.LogAttrs(
99+
context.Background(), slog.LevelDebug, "Backend update received",
100+
slog.String("backend", backend.Name),
101+
slog.Bool("serverChanged", serverChanged),
102+
slog.Bool("backendChangedOmitMetadata", backendChanged),
103+
slog.Bool("onlyServersUpdated", onlyServersUpdated),
104+
)
102105

103106
if err := configuration.EditStructuredBackend(backend.Name, &backend, c.activeTransaction, 0); err != nil {
104107
c.logger.LogAttrs(
@@ -110,7 +113,7 @@ func (c *clientNative) BackendEdit(backend models.Backend) error {
110113
}
111114

112115
// Servers only updated
113-
// Did we try runtime updates ? (server state update)
116+
// Did we try runtime updates? (server state update)
114117
if onlyServersUpdated {
115118
if reload.Instance().DynamicUpdateServerStateAttempted() {
116119
// Yes we did perform runtime update of server states
@@ -127,10 +130,9 @@ func (c *clientNative) BackendEdit(backend models.Backend) error {
127130
// We did not try runtime update (server create, for now is NOT done through runtime, it needs a reload)
128131
reload.Instance().SetReload("[onlyServersUpdated] [reload needed] - backend %s", backend.Name)
129132
}
130-
} else {
133+
} else if backendChanged { // backendChanged = false => only metadata changed. no need to reload.
131134
reload.Instance().SetReload("Backend upserted %s", backend.Name)
132135
}
133-
// Servers
134-
err = c.ServerReplaceAll(parser.Backends, backend.Name, backend.Servers)
135-
return err
136+
137+
return nil
136138
}

‎test/integration/httproute/httproute_basic_test.go‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,3 +236,129 @@ func (s *HTTPRouteTestSuite) Test_HTTPRoute_OK_Multiple_Listeners_One_Gateway()
236236
// For FE https
237237
s.ExpectMapContents(mapFilePath2, expectedMapsPath)
238238
}
239+
240+
// Test_HTTPRoute_OK_No_reload_When_Modification_Only_Affect_Maps check that if we change a route and it's only affects
241+
// maps (e.g. Path Prefix) it does not reload.
242+
//
243+
// the test scenario is the following:
244+
// - create a route with no endpoints and PathPrefix = /path1
245+
// - edit the route's path prefix to /path2 and check configuration is updated without reload and route condition is updated. updating route w/o endpoints allows testing conner cases
246+
// - scale up to 3 endpoints and check configuration is updated without reload.
247+
// - scale down to 1 endpoints and check configuration is updated without reload.
248+
// - update route path's PathPrefix = /path1 and check configuration is updated without reload and route condition is updated.
249+
func (s *HTTPRouteTestSuite) Test_HTTPRoute_OK_No_reload_When_Modification_Only_Affect_Maps() {
250+
fixtureDirPath := utils.GetCRDFixturePath()
251+
fixtureDir := "basic"
252+
253+
fixturePath := path.Join(fixtureDirPath, fixtureDir, "ok_no_reload_when_modification_only_affect_maps")
254+
s.CreateFixtures(fixturePath, []string{"gatewayclass.yaml", "gateway.yaml", "http-echo.yaml", "route-path-prefix-1.yaml"})
255+
mapFilePath1 := "hug_http_8080"
256+
mapFilePath2 := "hug_http_8088"
257+
defer s.CleanupFixturesCheckMapFiles(fixturePath, []string{"gatewayclass.yaml", "gateway.yaml", "http-echo.yaml", "route-path-prefix-1.yaml"}, []string{mapFilePath1, mapFilePath2})
258+
259+
// Expected Conditions
260+
expectationsPath := path.Join(fixturePath, "expectations")
261+
expectedCondPath := path.Join(expectationsPath, "route-gen-1-conditions.yaml")
262+
expectedConditions := s.YamlToRouteConditions(expectedCondPath)
263+
264+
httpRouteName := "route-echo"
265+
s.expectConditionsUpdated(s.Test().Ctx, s.Test().Namespace, httpRouteName, expectedConditions)
266+
267+
// Check AttachedRoutes on Gateway status
268+
s.expectAttachedRoute(s.Test().Ctx, s.Test().Namespace, "gateway", "http", 1)
269+
s.expectAttachedRoute(s.Test().Ctx, s.Test().Namespace, "gateway", "http2", 1)
270+
271+
// haproxy.cfg Backends
272+
const backendName = "hug_e2e-tests-httproute_http-echo_80__"
273+
expectedBackends := []string{backendName}
274+
s.ExpectBackends(s.Test().Ctx, path.Join(expectationsPath, "backends-gen-1"), expectedBackends)
275+
276+
// Check Maps for route-prefix-1 (ie pathPrefix = /path1)
277+
expectedMapsPathV1 := path.Join(expectationsPath, "maps-path-prefix-1")
278+
s.ExpectListenerRouteMapContents(expectedMapsPathV1)
279+
s.ExpectMapContents(mapFilePath1, expectedMapsPathV1) // For FE http
280+
s.ExpectMapContents(mapFilePath2, expectedMapsPathV1) // For FE https
281+
282+
// check Server
283+
s.ExpectServers(backendName, []string{})
284+
285+
// From now we should not have any reloads
286+
oldPid := s.WaitForNoReloadsAnyMore(10*time.Second, 2*time.Second)
287+
288+
s.T().Logf("====================================== Edit route to update Path Prefix (no endpoints) ======================================")
289+
s.CreateFixtures(fixturePath, []string{"route-path-prefix-2.yaml"}) // no need to clean up it the same obj as route-path-prefix-1.yaml
290+
291+
expectedConditionsV2 := s.YamlToRouteConditions(path.Join(expectationsPath, "route-gen-2-conditions.yaml"))
292+
s.expectConditionsUpdated(s.Test().Ctx, s.Test().Namespace, httpRouteName, expectedConditionsV2)
293+
294+
// haproxy.cfg Backends
295+
s.ExpectBackends(s.Test().Ctx, path.Join(expectationsPath, "backends-gen-2-ep-0"), expectedBackends)
296+
297+
// Check Maps-prefix-2
298+
expectedMapsPathV2 := path.Join(expectationsPath, "maps-path-prefix-2")
299+
s.ExpectListenerRouteMapContents(expectedMapsPathV2)
300+
s.ExpectMapContents(mapFilePath1, expectedMapsPathV2) // For FE http
301+
s.ExpectMapContents(mapFilePath2, expectedMapsPathV2) // For FE https
302+
303+
// check Server
304+
s.ExpectServers(backendName, []string{})
305+
306+
s.ConsistentlyNoReload(oldPid, 4*time.Second)
307+
308+
s.T().Logf("====================================== scale up endpoints to 3 ======================================")
309+
310+
// scale deploy to 3 pods
311+
s.CreateFixtures(fixturePath, []string{"echo-endpoints-1.yaml"})
312+
defer s.CleanupFixtures(fixturePath, []string{"echo-endpoints-1.yaml"})
313+
s.expectConditionsUpdated(s.Test().Ctx, s.Test().Namespace, httpRouteName, expectedConditionsV2)
314+
315+
// haproxy.cfg Backends
316+
s.ExpectBackends(s.Test().Ctx, path.Join(expectationsPath, "backends-gen-2-ep-3"), expectedBackends)
317+
318+
// Check Maps-prefix-2 (should be the same)
319+
s.ExpectListenerRouteMapContents(expectedMapsPathV2)
320+
s.ExpectMapContents(mapFilePath1, expectedMapsPathV2) // For FE http
321+
s.ExpectMapContents(mapFilePath2, expectedMapsPathV2) // For FE https
322+
323+
// check Server
324+
s.ExpectServers(backendName, []string{"SRV_4827409c6115096b8dde5db0092cea2f54213123", "SRV_8ec3713870978506a7ecded834e9907edcf2e619", "SRV_fe3f9ea252b531060fe66a137b37d38263502132"})
325+
326+
s.ConsistentlyNoReload(oldPid, 4*time.Second)
327+
328+
s.T().Logf("====================================== scale down endpoint to 1 ======================================")
329+
330+
// scale deploy to 1 pods
331+
s.CreateFixtures(fixturePath, []string{"echo-endpoints-2.yaml"}) // no need to clean up it's the same obj as scale=3
332+
333+
// haproxy.cfg Backends
334+
s.ExpectBackends(s.Test().Ctx, path.Join(expectationsPath, "backends-gen-2-ep-1"), expectedBackends)
335+
336+
// Check Maps-prefix-2 (should be the same)
337+
s.ExpectListenerRouteMapContents(expectedMapsPathV2)
338+
s.ExpectMapContents(mapFilePath1, expectedMapsPathV2) // For FE http
339+
s.ExpectMapContents(mapFilePath2, expectedMapsPathV2) // For FE https
340+
341+
// check Server
342+
s.ExpectServers(backendName, []string{"SRV_4827409c6115096b8dde5db0092cea2f54213123"})
343+
s.ConsistentlyNoReload(oldPid, 4*time.Second)
344+
345+
s.T().Logf("====================================== edit PathPrefix = path1 ======================================")
346+
s.CreateFixtures(fixturePath, []string{"route-path-prefix-1.yaml"}) // no need to clean up it the same obj as route-path-prefix-1.yaml
347+
expectedConditionsV3 := s.YamlToRouteConditions(path.Join(expectationsPath, "route-gen-3-conditions.yaml"))
348+
s.expectConditionsUpdated(s.Test().Ctx, s.Test().Namespace, httpRouteName, expectedConditionsV3)
349+
350+
// Check AttachedRoutes on Gateway status
351+
s.expectAttachedRoute(s.Test().Ctx, s.Test().Namespace, "gateway", "http", 1)
352+
s.expectAttachedRoute(s.Test().Ctx, s.Test().Namespace, "gateway", "http2", 1)
353+
354+
// haproxy.cfg Backends
355+
s.ExpectBackends(s.Test().Ctx, path.Join(expectationsPath, "backends-gen-3"), expectedBackends)
356+
357+
// Check Maps for route-path-prefix-1 (ie pathPrefix = /path1)
358+
s.ExpectListenerRouteMapContents(expectedMapsPathV1)
359+
s.ExpectMapContents(mapFilePath1, expectedMapsPathV1) // For FE http
360+
s.ExpectMapContents(mapFilePath2, expectedMapsPathV1) // For FE https
361+
362+
s.ExpectServers(backendName, []string{"SRV_4827409c6115096b8dde5db0092cea2f54213123"})
363+
s.ConsistentlyNoReload(oldPid, 4*time.Second)
364+
}

‎test/integration/httproute/manifests/basic/ok_no_reload_when_modification_only_affect_maps/echo-endpoints-1.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
apiVersion: discovery.k8s.io/v1
2+
kind: EndpointSlice
3+
metadata:
4+
name: http-echo
5+
labels:
6+
kubernetes.io/service-name: http-echo
7+
addressType: IPv4
8+
endpoints:
9+
- addresses:
10+
- 10.0.0.1
11+
- 10.0.0.2
12+
- 10.0.0.3
13+
conditions:
14+
ready: true
15+
serving: true
16+
terminating: false
17+
ports:
18+
- name: http
19+
port: 8888
20+
protocol: TCP
21+
- name: https
22+
port: 8443
23+
protocol: TCP

‎test/integration/httproute/manifests/basic/ok_no_reload_when_modification_only_affect_maps/echo-endpoints-2.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
apiVersion: discovery.k8s.io/v1
2+
kind: EndpointSlice
3+
metadata:
4+
name: http-echo
5+
labels:
6+
kubernetes.io/service-name: http-echo
7+
addressType: IPv4
8+
endpoints:
9+
- addresses:
10+
- 10.0.0.1
11+
conditions:
12+
ready: true
13+
serving: true
14+
terminating: false
15+
ports:
16+
- name: http
17+
port: 8888
18+
protocol: TCP
19+
- name: https
20+
port: 8443
21+
protocol: TCP

‎test/integration/httproute/manifests/basic/ok_no_reload_when_modification_only_affect_maps/expectations/backends-gen-1/hug_e2e-tests-httproute_http-echo_80__.yaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
from: haproxytech
2+
metadata:
3+
hug:
4+
HTTPRoute:
5+
e2e-tests-httproute/route-echo:
6+
Generation: 1
7+
LinkID: hug
8+
mode: http
9+
name: hug_e2e-tests-httproute_http-echo_80__
10+
abortonclose: disabled
11+
balance:
12+
algorithm: roundrobin
13+
default_server:
14+
check: enabled
15+
forwardfor:
16+
enabled: enabled
17+
server_timeout: 50000

‎test/integration/httproute/manifests/basic/ok_no_reload_when_modification_only_affect_maps/expectations/backends-gen-2-ep-0/hug_e2e-tests-httproute_http-echo_80__.yaml‎

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

‎test/integration/httproute/manifests/basic/ok_no_reload_when_modification_only_affect_maps/expectations/backends-gen-2-ep-1/hug_e2e-tests-httproute_http-echo_80__.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
from: haproxytech
2+
metadata:
3+
hug:
4+
HTTPRoute:
5+
e2e-tests-httproute/route-echo:
6+
Generation: 2
7+
LinkID: hug
8+
mode: http
9+
name: hug_e2e-tests-httproute_http-echo_80__
10+
abortonclose: disabled
11+
balance:
12+
algorithm: roundrobin
13+
default_server:
14+
check: enabled
15+
forwardfor:
16+
enabled: enabled
17+
server_timeout: 50000
18+
Servers:
19+
SRV_4827409c6115096b8dde5db0092cea2f54213123:
20+
Address: "10.0.0.1"
21+
Name: "SRV_4827409c6115096b8dde5db0092cea2f54213123"
22+
Port: 8888
23+
Maintenance: "disabled"

‎test/integration/httproute/manifests/basic/ok_no_reload_when_modification_only_affect_maps/expectations/backends-gen-2-ep-3/hug_e2e-tests-httproute_http-echo_80__.yaml‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
from: haproxytech
2+
metadata:
3+
hug:
4+
HTTPRoute:
5+
e2e-tests-httproute/route-echo:
6+
Generation: 2
7+
LinkID: hug
8+
mode: http
9+
name: hug_e2e-tests-httproute_http-echo_80__
10+
abortonclose: disabled
11+
balance:
12+
algorithm: roundrobin
13+
default_server:
14+
check: enabled
15+
forwardfor:
16+
enabled: enabled
17+
server_timeout: 50000
18+
Servers:
19+
SRV_4827409c6115096b8dde5db0092cea2f54213123:
20+
Address: "10.0.0.1"
21+
Name: "SRV_4827409c6115096b8dde5db0092cea2f54213123"
22+
Port: 8888
23+
Maintenance: "disabled"
24+
SRV_fe3f9ea252b531060fe66a137b37d38263502132:
25+
Address: "10.0.0.2"
26+
Name: "SRV_fe3f9ea252b531060fe66a137b37d38263502132"
27+
Port: 8888
28+
Maintenance: "disabled"
29+
SRV_8ec3713870978506a7ecded834e9907edcf2e619:
30+
Name: "SRV_8ec3713870978506a7ecded834e9907edcf2e619"
31+
Port: 8888
32+
Address: "10.0.0.3"
33+
Maintenance: "disabled"
34+

‎test/integration/httproute/manifests/basic/ok_no_reload_when_modification_only_affect_maps/expectations/backends-gen-3/hug_e2e-tests-httproute_http-echo_80__.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
from: haproxytech
2+
metadata:
3+
hug:
4+
HTTPRoute:
5+
e2e-tests-httproute/route-echo:
6+
Generation: 3
7+
LinkID: hug
8+
mode: http
9+
name: hug_e2e-tests-httproute_http-echo_80__
10+
abortonclose: disabled
11+
balance:
12+
algorithm: roundrobin
13+
default_server:
14+
check: enabled
15+
forwardfor:
16+
enabled: enabled
17+
server_timeout: 50000
18+
Servers:
19+
"SRV_4827409c6115096b8dde5db0092cea2f54213123":
20+
Address: "10.0.0.1"
21+
Name: "SRV_4827409c6115096b8dde5db0092cea2f54213123"
22+
Port: 8888
23+
Maintenance: "disabled"

‎test/integration/httproute/manifests/basic/ok_no_reload_when_modification_only_affect_maps/expectations/maps-path-prefix-1/hug_http_8080/listener_route_exact_match.map‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
e2e-tests-httproute/gateway_http/example.haproxy e2e-tests-httproute/gateway_http/e2e-tests-httproute/route-echo

0 commit comments

Comments
 (0)