Skip to content

fix: surface Gateway API route translation failures in status (backport apache/apisix-ingress-controller#2898) - #493

Merged
AlinsRan merged 1 commit into
masterfrom
fix/route-update-error-status
Oct 12, 2026
Merged

AlinsRan merged 1 commit into
masterfrom
fix/route-update-error-status

Conversation

@AlinsRan

@AlinsRan AlinsRan commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

The five Gateway API route controllers wrote Accepted=True before calling Provider.Update, so a translation or buildConfig error only caused a requeue and never showed up on the route status:

// internal/controller/httproute_controller.go (before)
UpdateStatus(r.Updater, r.Log, tctx)

if isRouteAccepted(gateways) && err == nil {
	...
	if err := r.Provider.Update(ctx, tctx, routeToUpdate); err != nil {
		return ctrl.Result{}, err
	}

This change calls Provider.Update first. On failure the route gets Accepted=False with the error message, and the reconcile still returns the error so it is retried. The next successful reconcile writes Accepted=True again. Applies to HTTPRoute, GRPCRoute, TCPRoute, UDPRoute and TLSRoute.

Upstream issue: apache/apisix-ingress-controller#2840

Differences from upstream

The test scaffolding differs from upstream, so the tests were adapted rather than cherry-picked:

  • recordingProvider lives in httproute_controller_test.go here; updateErr was added there instead of in apisixconsumer_controller_test.go.
  • newHTTPRouteRetractFixture now also returns the shared recordingUpdater.

Known follow-ups (same as upstream review)

  • The failure reason is still Accepted, because SetRouteConditionAccepted hardcodes it; it should be SyncFailed.
  • status.go still has no KindTLSRoute case, so asynchronous sync failures of TLSRoute remain invisible.

Tests

  • TestHTTPRouteReconcile_UpdateErrorIsVisibleOnStatus and TestGRPCRouteReconcile_UpdateErrorIsVisibleOnStatus fail on master and pass with this change.
  • go test -race ./internal/controller/ passes.

Summary by CodeRabbit

  • Bug Fixes
    • Route acceptance status now reflects whether the route was successfully applied. If an update fails, the route is marked as unaccepted with the failure message, and the error is returned.
    • Routes that are not accepted continue through the existing deletion behavior.

@CLAassistant

CLAassistant commented Oct 10, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 166dbcd5-5115-4146-8401-9721d45c330e

📥 Commits

Reviewing files that changed from the base of the PR and between df743e4 and d09829d.


📒 Files selected for processing (8)
  • internal/controller/grpcroute_controller.go
  • internal/controller/grpcroute_controller_test.go
  • internal/controller/httproute_controller.go
  • internal/controller/httproute_controller_retract_test.go
  • internal/controller/httproute_controller_test.go
  • internal/controller/tcproute_controller.go
  • internal/controller/tlsroute_controller.go
  • internal/controller/udproute_controller.go

Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.



📝 Walkthrough

Walkthrough

The GRPC, HTTP, TCP, TLS, and UDP route reconcilers now update the provider before writing route status. If an update fails, they record the error in the Accepted condition and return the error after status processing.

Changes

Route reconcile status

Layer / File(s) Summary
Provider sync before status
internal/controller/grpcroute_controller.go, internal/controller/httproute_controller.go, internal/controller/tcproute_controller.go, internal/controller/tlsroute_controller.go, internal/controller/udproute_controller.go
Accepted routes call Provider.Update before status is written. An update failure sets Accepted to false with the error message. The reconciler returns the error after status processing.
Route status test coverage
internal/controller/grpcroute_controller_test.go, internal/controller/httproute_controller_retract_test.go, internal/controller/httproute_controller_test.go
Tests check Accepted status for successful and failed provider updates. The recording provider can return a configured update error.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: shreemaan-abhishek


Merge Risk: ⚪ Minimal · up to d0982

Provider update failures are reported in route status and retried. No actionable merge-blocking issue is established by the supplied evidence.

Pre-merge checks | Passed 5 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review Warning ❌ Blocking: the PR adds only controller unit tests. The fixtures use controller-runtime's fake.NewClientBuilder, recordingProvider, and recordingUpdater (`internal/controller/grpcroute_controlle… Add E2E tests using the existing test/e2e/gatewayapi scaffold. Apply GatewayClass, Gateway, and each affected route through the Kubernetes API, trigger a deterministic translation or Provider.Update failure, and poll the persisted route…
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Security Check Passed No security issue was introduced. Category 1 — No issues found: the changed controllers copy Provider.Update error text into route status, but reviewed provider and translator errors expose validation…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: exposing Gateway API route translation failures in route status. The backport reference provides useful context.

Full details: E2e Test Quality Review

Explanation

❌ Blocking: the PR adds only controller unit tests. The fixtures use controller-runtime's fake.NewClientBuilder, recordingProvider, and recordingUpdater (internal/controller/grpcroute_controller_test.go:84-101,117-132; internal/controller/httproute_controller_retract_test.go:97-118,183-196). No E2E or integration test file is changed. These tests do not exercise the deployed controller, Kubernetes API status persistence, the real provider, or the data plane. Coverage is also limited to HTTPRoute and GRPCRoute, while the implementation changes TCPRoute, UDPRoute, and TLSRoute. The assertions are readable and the unit-test error paths are checked, but they do not satisfy the required full business-flow coverage.

Resolution

Add E2E tests using the existing test/e2e/gatewayapi scaffold. Apply GatewayClass, Gateway, and each affected route through the Kubernetes API, trigger a deterministic translation or Provider.Update failure, and poll the persisted route status for Accepted=False with the failure message. Then correct the route, verify retry and Accepted=True, and verify the expected data-plane result. Cover HTTPRoute, GRPCRoute, TCPRoute, UDPRoute, and TLSRoute, or use a clearly parameterized test that exercises all five. Keep the current unit tests as fast regression tests.



  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Call Provider.Update before persisting Accepted=True so a translation
or buildConfig error is visible on HTTPRoute, GRPCRoute, TCPRoute,
UDPRoute, and TLSRoute instead of a green status plus requeue.
@AlinsRan
AlinsRan force-pushed the fix/route-update-error-status branch from 8ef379f to d09829d Compare October 10, 2026 01:36
@github-actions

Copy link
Copy Markdown
Contributor

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-10-10T01:44:01Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  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: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidNonExistentBackendRef
    - HTTPRouteListenerHostnameMatching
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    statistics:
      Failed: 0
      Passed: 30
      Skipped: 7
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 7 test skips. Extended tests partially
    succeeded with 1 test skips.
- core:
    result: partial
    skippedTests:
    - GRPCRouteListenerHostnameMatching
    statistics:
      Failed: 0
      Passed: 14
      Skipped: 1
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests partially succeeded with 1 test skips. Extended tests succeeded.
- core:
    result: partial
    skippedTests:
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 16
      Skipped: 4
  extended:
    result: partial
    skippedTests:
    - TLSRouteTerminateSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 3
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 4 test skips. Extended tests partially
    succeeded with 1 test skips.
succeededProvisionalTests:
- GatewayOptionalAddressValue

@github-actions

Copy link
Copy Markdown
Contributor

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-10-10T01:45:27Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  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: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidNonExistentBackendRef
    - HTTPRouteListenerHostnameMatching
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    statistics:
      Failed: 0
      Passed: 30
      Skipped: 7
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 7 test skips. Extended tests partially
    succeeded with 1 test skips.
- core:
    result: partial
    skippedTests:
    - GRPCRouteListenerHostnameMatching
    statistics:
      Failed: 0
      Passed: 14
      Skipped: 1
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests partially succeeded with 1 test skips. Extended tests succeeded.
- core:
    result: partial
    skippedTests:
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 16
      Skipped: 4
  extended:
    result: partial
    skippedTests:
    - TLSRouteTerminateSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 3
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 4 test skips. Extended tests partially
    succeeded with 1 test skips.
succeededProvisionalTests:
- GatewayOptionalAddressValue

@github-actions

Copy link
Copy Markdown
Contributor

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2026-10-10T02:04:00Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  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
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 3
      Passed: 33
      Skipped: 1
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests failed with 3 test failures. Extended tests partially succeeded
    with 1 test skips.
- core:
    failedTests:
    - GatewayModifyListeners
    result: failure
    statistics:
      Failed: 1
      Passed: 14
      Skipped: 0
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests failed with 1 test failures. Extended tests succeeded.
- core:
    failedTests:
    - GatewayModifyListeners
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    result: failure
    statistics:
      Failed: 5
      Passed: 15
      Skipped: 0
  extended:
    failedTests:
    - TLSRouteTerminateSimpleSameNamespace
    result: failure
    statistics:
      Failed: 1
      Passed: 3
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests failed with 5 test failures. Extended tests failed with 1 test
    failures.
succeededProvisionalTests:
- GatewayOptionalAddressValue

@AlinsRan AlinsRan self-assigned this Oct 12, 2026
@AlinsRan
AlinsRan merged commit 47b802c into master Oct 12, 2026
22 checks passed
@AlinsRan
AlinsRan deleted the fix/route-update-error-status branch October 12, 2026 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants