Conversation
ron96g
added
performance
f4d1585 to
70ceeae
Compare
Measured impact of this PR (identical burst test)Continuing the reproduction methodology from PR #662's status quo comment: same Note: this branch does not include the watch-predicate fix from #662, so the underlying reconcile fan-out (every Route update still enqueues all subscribing ConsumeRoutes) is still present. This test isolates the effect of the Kong-write dedup alone. Kong write amplification — eliminatedNew metric
Zero incremental writes were sent to Kong's Admin API for any of these — confirming the diff-before-write logic works correctly under sustained churn, not just idle-state. Route controller's own queue wait — improved ~27x
Skipping the redundant Kong HTTP round-trips makes each Route reconcile complete much faster, freeing the worker sooner — so the Route controller's own queue backs up far less even under the same event volume. ConsumeRoute controller — still backed up (expected)ConsumeRoute's average queue wait was still ~6.9s (sum=1,796,938s / count=261,835) during this run. This is expected: this PR doesn't change which events are admitted into the queue, only how expensive processing each admitted item is. The fan-out itself (one Route update → reconcile of all subscribers) is only fixed by #662's watch predicates. TakeawayThis PR and #662 address two independent parts of the same regression:
Combining both should give the best result — happy to run a combined build next if useful. A small number of |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical reconciliation correctness issues remain in plugin bindings and upstream target pagination/selection.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (4)
What changed in this PR
This pull request refactors Kong reconciliation to avoid redundant writes while normalizing state, adding metrics, and expanding test coverage.
Changes:
- Adds entity-specific reconciliation with GET/project/compare/write logic.
- Normalizes Kong defaults, unordered values, targets, and plugin configuration.
- Adds reconciliation metrics and regenerated mocks.
- Updates dependencies, tooling, circuit-breaker integration, and tests.
| File | Description |
|---|---|
tools/snapshotter/go.sum |
Updates dependency checksums. |
tools/snapshotter/go.mod |
Adds indirect dependency metadata. |
gateway/tools/mockery.yaml |
Configures mock generation. |
gateway/pkg/kong/client/upstream.go |
Reconciles upstreams and targets. |
gateway/pkg/kong/client/upstream_test.go |
Tests upstream reconciliation. |
gateway/pkg/kong/client/route.go |
Reconciles services and routes. |
gateway/pkg/kong/client/route_test.go |
Tests route reconciliation. |
gateway/pkg/kong/client/response.go |
Handles Kong responses. |
gateway/pkg/kong/client/reconcile.go |
Provides generic reconciliation logic. |
gateway/pkg/kong/client/reconcile_test.go |
Tests reconciliation and normalization. |
gateway/pkg/kong/client/plugin/suite_test.go |
Tests plugin encoding. |
gateway/pkg/kong/client/plugin/encode.go |
Encodes plugin string maps. |
gateway/pkg/kong/client/plugin.go |
Reconciles plugins. |
gateway/pkg/kong/client/plugin_test.go |
Tests plugin reconciliation. |
gateway/pkg/kong/client/normalize.go |
Normalizes Kong representations. |
gateway/pkg/kong/client/mock/mock_KongClient.go |
Regenerates client mocks. |
gateway/pkg/kong/client/mock/mock_KongAdminApi.go |
Regenerates Admin API mocks. |
gateway/pkg/kong/client/metrics.go |
Adds reconciliation metrics. |
gateway/pkg/kong/client/metrics_test.go |
Tests reconciliation metrics. |
gateway/pkg/kong/client/kongclient.go |
Defines client interfaces. |
gateway/pkg/kong/client/helpers_test.go |
Adds shared test helpers. |
gateway/pkg/kong/client/error.go |
Refines Kong error handling. |
gateway/pkg/kong/client/error_test.go |
Tests error handling. |
gateway/pkg/kong/client/consumer.go |
Reconciles consumers and memberships. |
gateway/pkg/kong/client/consumer_test.go |
Tests consumer reconciliation. |
gateway/pkg/kong/client/client.go |
Removes the monolithic implementation. |
gateway/pkg/kong/client/client_test.go |
Removes obsolete tests. |
gateway/pkg/kong/api/client.gen.go |
Regenerates Kong API client code. |
gateway/Makefile |
Updates generation and lint tooling. |
gateway/internal/features/mock/mock_FeaturesBuilder.go |
Updates generated mocks. |
gateway/internal/features/mock/mock_Feature.go |
Updates generated mocks. |
gateway/internal/features/feature/circuit_breaker.go |
Delegates upstream reconciliation. |
gateway/internal/features/feature/circuit_breaker_test.go |
Tests circuit-breaker integration. |
gateway/internal/controller/suite_test.go |
Updates mock setup. |
gateway/go.mod |
Adds Prometheus and test dependencies. |
Files not reviewed (5)
- gateway/internal/features/mock/mock_Feature.go: Generated file
- gateway/internal/features/mock/mock_FeaturesBuilder.go: Generated file
- gateway/pkg/kong/api/client.gen.go: Generated file
- gateway/pkg/kong/client/mock/mock_KongAdminApi.go: Generated file
- gateway/pkg/kong/client/mock/mock_KongClient.go: Generated file
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| desired.Consumer, desired.Route, desired.Service = nil, nil, nil | ||
| current.Consumer, current.Route, current.Service = nil, nil, nil |
| if e.tags != nil && len(*e.tags) > 0 { | ||
| tags := strings.Join(*e.tags, ",") | ||
| params.Tags = &tags | ||
| } |
| if params.Offset != nil && *response.JSON200.Offset == *params.Offset { | ||
| return nil, false, fmt.Errorf("target list pagination offset did not advance") | ||
| } | ||
| params.Offset = response.JSON200.Offset |
| slices.Sort(entries) | ||
| return json.Marshal(entries) |
Summary
Avoid redundant Kong Admin API writes during Gateway reconciliation.
The Kong client now reads the current Service, Route, Consumer, Plugin, Upstream, and Target state, normalizes it to the controller-owned representation, and writes only when the desired configuration differs. This preserves correction of out-of-band Kong changes while preventing no-op upserts during periodic resyncs and duplicate reconciles.
Changes
• Split the former pkg/kong/client/client.go into entity-specific client files.
• Add reusable GET → project → compare → write reconciliation logic.
• Normalize Kong defaults, JSON value types, and unordered config lists to prevent false differences.
• Reconcile circuit-breaker upstream targets instead of appending identical targets.
• Keep consumer-group membership checks, even when Consumer upserts are skipped.
• Add gateway_kong_reconcile_total{entity,outcome} to measure writes, no-op reconciliations, and errors.
• Sort StringMap JSON entries for deterministic plugin configuration.
• Add comprehensive tests for unchanged, changed, missing, reordered, and invalid Kong responses.