fix: rebuild the ADC baseline on leader acquisition#430
Open
AlinsRan wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughThe ADC version requirement is updated, sync requests gain conditional cache bypassing, and standalone clients rebuild baselines after leadership changes or ChangesADC cache recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Provider
participant ADCClient
participant Executor
participant APISIX
Provider->>ADCClient: InvalidateADCCache()
ADCClient->>Executor: Execute sync with BypassCache
Executor->>APISIX: Submit sync request
APISIX-->>ADCClient: Return sync result or conf_version rejection
ADCClient->>Executor: Retry with BypassCache=true
Executor->>APISIX: Rebuild baseline and submit sync
APISIX-->>ADCClient: Return retry result
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/adc/client/client.go`:
- Around line 397-419: Extract the stale conf_version recovery sequence
currently in the per-config path into a shared helper, preserving its logging,
metric recording, cache bypass, retry, and error-reporting behavior. Invoke this
helper for the global-rule ADC execution before that branch returns, as well as
from the existing config path, so both execution paths recover from conflicts.
Add a regression test covering global_rule synchronization with a stale
conf_version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5113521-b802-433f-b553-212fb9476afb
📒 Files selected for processing (8)
Makefileapi/adc/types.godocs/en/latest/developer-guide.mdinternal/adc/client/client.gointernal/adc/client/executor.gointernal/adc/client/executor_test.gotest/e2e/apisix/conf_version.gotest/e2e/scaffold/apisix_deployer.go
Contributor
conformance test report - apisix-standalone modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-14T05:54:17Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
result: partial
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 0
Passed: 32
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests partially succeeded with 1 test skips. Extended tests partially
succeeded with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
Contributor
conformance test report - apisix modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-14T05:54:21Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
failedTests:
- HTTPRouteInvalidBackendRefUnknownKind
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
Contributor
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-14T06:11:55Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
failedTests:
- GatewayModifyListeners
result: failure
statistics:
Failed: 1
Passed: 11
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests failed with 1 test failures.
- core:
failedTests:
- GatewayModifyListeners
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips.
- core:
failedTests:
- GatewayModifyListeners
- TLSRouteSimpleSameNamespace
result: failure
statistics:
Failed: 2
Passed: 9
Skipped: 0
name: GATEWAY-TLS
summary: Core tests failed with 2 test failures. |
AlinsRan
force-pushed
the
fix/adc-conf-version-rebuild
branch
from
a9343b4 to
e0a7361
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/adc/client/client.go (1)
384-407: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftGlobal-rule sync path still has no conflict recovery.
This branch executes ADC (
c.executor.Execute(ctx, config, args)) and handles its error, but unlike the non-global path added at lines 440-483, it never setsconfig.BypassCachebased on baseline currency and never retries viarejectedByDataPlane. Any config whose task includesglobal_rulewill still permanently fail on a staleconf_versioninstead of self-healing. This was already flagged on a prior commit of this file; it remains unaddressed in the current diff.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/adc/client/client.go` around lines 384 - 407, Update the global-rule execution flow around c.executor.Execute and align it with the non-global path’s conflict recovery: set config.BypassCache based on baseline currency, then retry rejected stale-conf_version executions through rejectedByDataPlane. Preserve the existing status, error collection, and execution-duration metrics while allowing global_rule tasks to self-heal.internal/adc/client/executor.go (1)
424-448: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrefer a 400 data-plane status when aggregating standalone endpoint failures.
dataPlaneStatuscurrently takes the first failing endpoint’s status, so a later400 Bad Requestcan be hidden behind an earlier non-400 failure and skip the bypassCache retry inrejectedByDataPlane.🐛 Proposed fix
for _, ep := range result.EndpointStatus { if !ep.Success { failedEndpoints = append(failedEndpoints, fmt.Sprintf("%s: %s", ep.Server, ep.Reason)) - if dataPlaneStatus == 0 { + if dataPlaneStatus == 0 || ep.Response.Status == http.StatusBadRequest { dataPlaneStatus = ep.Response.Status } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/adc/client/executor.go` around lines 424 - 448, Update the endpoint-status aggregation in the standalone-mode handling to prefer HTTP 400 when selecting dataPlaneStatus. In the loop over result.EndpointStatus, continue recording a failure status, but allow a later failing endpoint with status 400 to replace an earlier non-400 status so rejectedByDataPlane can trigger its bypassCache retry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/adc/client/client.go`:
- Around line 384-407: Update the global-rule execution flow around
c.executor.Execute and align it with the non-global path’s conflict recovery:
set config.BypassCache based on baseline currency, then retry rejected
stale-conf_version executions through rejectedByDataPlane. Preserve the existing
status, error collection, and execution-duration metrics while allowing
global_rule tasks to self-heal.
In `@internal/adc/client/executor.go`:
- Around line 424-448: Update the endpoint-status aggregation in the
standalone-mode handling to prefer HTTP 400 when selecting dataPlaneStatus. In
the loop over result.EndpointStatus, continue recording a failure status, but
allow a later failing endpoint with status 400 to replace an earlier non-400
status so rejectedByDataPlane can trigger its bypassCache retry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2716049f-d5cb-4be4-8d9b-9c8d6fc3b394
📒 Files selected for processing (10)
Makefileapi/adc/types.godocs/en/latest/developer-guide.mdinternal/adc/client/client.gointernal/adc/client/executor.gointernal/adc/client/executor_test.gointernal/provider/apisix/provider.gointernal/types/error.gotest/e2e/apisix/conf_version.gotest/e2e/scaffold/apisix_deployer.go
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/en/latest/developer-guide.md
- Makefile
- test/e2e/apisix/conf_version.go
- test/e2e/scaffold/apisix_deployer.go
AlinsRan
force-pushed
the
fix/adc-conf-version-rebuild
branch
from
e0a7361 to
52d98df
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/adc/client/client.go`:
- Around line 435-475: The baseline check, execution, and mark-current sequence
in applySync must be serialized per cache key. Replace the shared-lock
protection used by applySync with an exclusive lock or equivalent per-key guard
covering baselineIsCurrent, Execute (including retry), and markBaselineCurrent,
while preserving concurrency for different keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 214a3c9d-6fec-44ae-b2fb-d75898cf4969
📒 Files selected for processing (9)
Makefileapi/adc/types.godocs/en/latest/developer-guide.mdinternal/adc/client/client.gointernal/adc/client/executor.gointernal/adc/client/executor_test.gointernal/provider/apisix/provider.gotest/e2e/apisix/conf_version.gotest/e2e/scaffold/apisix_deployer.go
🚧 Files skipped from review as they are similar to previous changes (6)
- Makefile
- internal/provider/apisix/provider.go
- test/e2e/apisix/conf_version.go
- test/e2e/scaffold/apisix_deployer.go
- internal/adc/client/executor.go
- docs/en/latest/developer-guide.md
Comment on lines
+435
to
+475
| // The ADC sidecar outlives the controller process, so the baseline it holds for this | ||
| // cacheKey may be one an earlier leadership term left behind. Re-derive it from the | ||
| // data plane the first time this term syncs the key, before it can be pushed from. | ||
| standalone := config.BackendType == backendAPISIXStandalone | ||
| config.BypassCache = standalone && !c.baselineIsCurrent(config.Name) | ||
|
|
||
| err := c.executor.Execute(ctx, config, args) | ||
|
|
||
| // Rebuilding on leader acquisition covers where staleness comes from. The safety net | ||
| // covers what it cannot foresee -- another writer on this data plane, a desync no | ||
| // leadership change explains -- and a conf_version the data plane refuses is the only | ||
| // way any of that shows itself. Re-read the data plane and push again. | ||
| if standalone && !config.BypassCache && isConfVersionRejection(err) { | ||
| c.log.Info("data plane rejected a stale conf_version, rebuilding the ADC baseline", | ||
| "config", config.Name, "error", err.Error()) | ||
| // Keep the rejection visible even when the sync recovers. The rebuild is not rate | ||
| // limited, so a rejection on every sync -- someone else writing to this data plane | ||
| // -- turns every sync into a full fetch and diff, and this counter is what says so. | ||
| pkgmetrics.RecordExecutionError(config.Name, "conf_version_conflict") | ||
|
|
||
| config.BypassCache = true | ||
| retryErr := c.executor.Execute(ctx, config, args) | ||
| if retryErr != nil && retryErr.Error() != err.Error() { | ||
| // Report the rejection as well. On its own a failed rebuild says nothing about | ||
| // what it was rebuilding for, and it is the rejection that names the cause -- | ||
| // an ADC server too old to know bypassCache, say, answers with a schema error | ||
| // that points nowhere near it. Unless the rebuild was rejected the same way, | ||
| // in which case saying it twice only pads the status message. | ||
| var rejected types.ADCExecutionError | ||
| if errors.As(err, &rejected) { | ||
| errs.Errors = append(errs.Errors, rejected) | ||
| } | ||
| } | ||
| err = retryErr | ||
| } | ||
|
|
||
| // Only a sync ADC accepted proves its baseline is now derived from the data plane. | ||
| if err == nil && config.BypassCache { | ||
| c.markBaselineCurrent(config.Name) | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all call sites of Client.sync / Client.Sync and check whether syncMu guards them.
rg -n --type=go -C3 '\.sync\(|\.Sync\(' internal/ | rg -v '_test.go'
rg -n --type=go -C3 'syncMu' internal/adc/client/Repository: api7/api7-ingress-controller
Length of output: 5354
🏁 Script executed:
#!/bin/bash
sed -n '150,330p' internal/adc/client/client.go | cat -nRepository: api7/api7-ingress-controller
Length of output: 6309
🏁 Script executed:
#!/bin/bash
set -e
rg -n --type=go -C2 '\.(Update|Delete|UpdateConfig|DeleteConfig)\(' internal/ | rg -v '_test.go'
printf '\n---\n'
sed -n '1,220p' internal/provider/apisix/provider.go | cat -n
printf '\n---\n'
sed -n '1,220p' internal/provider/api7ee/provider.go | cat -nRepository: api7/api7-ingress-controller
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -e
sed -n '330,520p' internal/adc/client/client.go | cat -n
printf '\n---\n'
rg -n --type=go 'baselineIsCurrent|markBaselineCurrent|BypassCache|syncMu' internal/adc/client/client.goRepository: api7/api7-ingress-controller
Length of output: 8402
🏁 Script executed:
#!/bin/bash
set -e
printf 'client helpers:\n'
sed -n '90,120p' internal/adc/client/client.go | cat -n
printf '\nprovider locks:\n'
rg -n --type=go 'Lock\(|Unlock\(|syncCh|syncNotify|applySync\(' internal/provider/apisix/provider.go internal/provider/api7ee/provider.goRepository: api7/api7-ingress-controller
Length of output: 2847
syncMu still allows concurrent baseline rebuilds. applySync() takes only RLock(), so two Update/Delete calls can overlap baselineIsCurrent(name) → Execute() → markBaselineCurrent(name) for the same cache key and both push with BypassCache=true. If that first-rebuild invariant matters, serialize that sequence with an exclusive lock or per-key guard.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/adc/client/client.go` around lines 435 - 475, The baseline check,
execution, and mark-current sequence in applySync must be serialized per cache
key. Replace the shared-lock protection used by applySync with an exclusive lock
or equivalent per-key guard covering baselineIsCurrent, Execute (including
retry), and markBaselineCurrent, while preserving concurrency for different
keys.
The ADC server runs as a sidecar that outlives the controller process. Losing the
lease terminates the manager container but not the sidecar, so the baseline ADC
holds per cacheKey -- the last synced content plus the conf_version it generated --
survives a leadership change. Once the pod is elected again, ADC diffs against that
frozen snapshot and carries over conf_versions older than the ones the leader in
between has already pushed. APISIX standalone requires *_conf_version to be
monotonic and rejects the request:
upstreams_conf_version must be greater than or equal to (1779434128737)
The data plane validates the configuration as a whole, so every later sync keeps
failing and no update lands at all, endpoint updates included. Backends running a
single pod then return 502/504 once that pod is rescheduled, while multi-pod
backends keep serving on their remaining endpoints.
Winning the election is the one moment a baseline from an earlier term can enter the
picture, so that is where to answer it: the provider, which only runs once elected,
puts every baseline in doubt, and the first sync of each cacheKey re-derives it from
the data plane through ADC's bypassCache before anything is pushed from it. Only a
sync ADC accepts settles the question, so a rebuild that never landed is attempted
again rather than assumed.
Prevention cannot cover a desync no leadership change explains -- another writer on
this data plane, something we did not foresee -- and the only way any of that shows
itself is the data plane refusing a conf_version. So that rejection rebuilds the
baseline and pushes once more. It is recognised by the field it names rather than by
the sentence around it: conf_version belongs to the standalone admin API, we send
those keys ourselves, while the prose around them is APISIX's to reword. This is a
net, not the fix: nothing else rebuilds on it, and the reported bug does not come
back if it ever stops firing.
bypassCache needs ADC >= 0.27.0, so bump ADC_VERSION to 0.27.1. The sidecar image
the chart deploys has to move with it, or the rebuild is answered with a schema
error instead of healing.
The e2e bumps every *_conf_version the data plane holds straight through its Admin
API -- what the leader in between would have left behind -- and then expects an
Ingress update to still reach the data plane. Nothing restarts during the spec, so
what it exercises is the safety net. It reproduces the bug without the fix.
AlinsRan
force-pushed
the
fix/adc-conf-version-rebuild
branch
from
52d98df to
817216d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The ADC server runs as a sidecar that outlives the controller process. Losing the lease terminates the
managercontainer (controller-runtime makesmgr.Start()returnleader election lost, which exits the process) but not the sidecar, so ADC's per-cacheKeystate survives a leadership change.That state is the baseline ADC diffs against: the last synced content, plus the
conf_versionit generated.Operator.sync()clones the payload from that cachedrawConfigand only refreshes<type>_conf_versionfor the resource types the current diff touches — every other type carries the version stored in the cache.So when a pod is elected again:
*_conf_versionin APISIX.*_conf_versionto be monotonic — rejects it:The data plane validates the configuration as a whole, so the rejection is not limited to the offending resource type: every later sync keeps failing and no update lands at all, endpoint updates included. Backends running a single pod then return 502/504 as soon as that pod is rescheduled, while multi-pod backends keep serving on their remaining endpoints. It also explains why restarting the pod clears it (the sidecar dies with it) while the manager process restarting on its own does not.
Fix
ADC >= 0.27.0 accepts
bypassCacheon the/synctask (api7/adc#460):dump()then invalidates the cacheKey, re-fetches the configuration from the data plane, and re-seeds its version floor from the*_conf_versionvalues actually held there. A baseline is rebuilt exactly twice:On leader acquisition — this is the fix. Winning the election is the one moment a baseline from an earlier term can enter the picture, so that is where it is answered.
apisixProvider.Start()only runs once elected (NeedLeaderElection() == true), and it puts every baseline in doubt; the first sync of each cacheKey then re-derives it from the data plane before anything is pushed from it. Only a sync ADC accepts settles the question, so a rebuild that never landed — data plane not up yet, network gone — is attempted again rather than assumed. This depends on nothing but our own lifecycle.On a rejected
conf_version— this is a safety net. Prevention assumes a stale baseline can only come from a leadership change. What it cannot foresee — another writer on this data plane, something we did not anticipate — shows itself the same way: the data plane refuses the push. So that rejection rebuilds the baseline and pushes once more. It is recognised by the field it names, not by the sentence around it:conf_versionbelongs to the standalone admin API and we send those keys ourselves, while the prose around them is APISIX's to reword. Nothing else rebuilds on it, so if it ever stopped firing the bug would not come back with it.Everything else is left alone. A connection that never reached the data plane, a configuration the data plane refuses on its merits (an invalid plugin, say) — neither is a question re-deriving the baseline can answer, and both are reported exactly as before. Outside standalone mode
conf_versiondoes not exist and none of this runs. In the steady state the request body is byte for byte what it was:bypassCacheisomitempty, so a sync that is not rebuilding does not carry it at all.ADC_VERSIONgoes to 0.27.1, the earliest release withbypassCache(0.28.0 also flips the differ to v4 by default, which is out of scope here).Important
The ADC sidecar image must move too. Deployments get their sidecar tag from the chart, not from this repo. A controller running against an ADC older than 0.27.0 has its rebuild answered with a schema error instead of healing — the fix becomes a no-op. A matching chart bump has to ship with this.
Worth calling out for review: a conflict is no longer fail-closed. Today a rejected
conf_versionstops the controller from writing. It now rebuilds and pushes anyway, so a second party writing to the same data plane gets overwritten rather than backed off from. The alternative it backs off into is a permanently broken ingress, so overwriting is the semantic we want — and the conflict is counted (conf_version_conflict) so a persistent ping-pong shows up in metrics rather than silently. The rebuild is not rate limited, and that counter is what says so.Test
E2E (
test/e2e/apisix/conf_version.go) bumps every*_conf_versionthe data plane holds straight through its Admin API — what the leader in between would have left behind — and then expects an Ingress update to still reach the data plane. Nothing restarts during the spec, so the baseline rebuilt at election is already current by then: what it exercises is the safety net. Without the fix it reproduces the failure, the sync carrying over the versions of the resource types its diff does not touch:Verified against APISIX 3.17.0 and ADC 0.27.1: the spec passes with the fix and fails without it. It needs the data plane Admin API reachable from a spec, so
APISIXDeployernow exposes a client for the tunnel the scaffold already forwards.Unit tests cover the rest: the rebuild happens once per cacheKey per term and not again; a rebuild ADC never accepted is retried rather than recorded; the rejection is recognised however it is worded; an unreachable data plane and an invalid plugin configuration do not trigger one;
bypassCachereaches the/syncbody, stays off the/validatebody (its task schema rejects unknown fields), never writes back into the config theConfigManagerholds; the retry happens once rather than in a loop; a failed rebuild reports both the rejection and the reason it could not answer it, without saying the same thing twice.Summary by CodeRabbit
Bug Fixes
Testing
Documentation