Skip to content

fix: exclude status-only API v2 resources from readiness gating#411

Open
AlinsRan wants to merge 1 commit intomasterfrom
backport/pr-2745
Open

fix: exclude status-only API v2 resources from readiness gating#411
AlinsRan wants to merge 1 commit intomasterfrom
backport/pr-2745

Conversation

@AlinsRan
Copy link
Copy Markdown
Contributor

@AlinsRan AlinsRan commented May 8, 2026

Summary

  • exclude ApisixPluginConfig and ApisixUpstream from API v2 startup readiness gating
  • keep readiness registration limited to resources that actually drive startup sync progression
  • add a unit test to lock the readiness resource list and prevent regressions

Why

ApisixPluginConfig and ApisixUpstream are status-oriented resources in this branch and do not call Readier.Done(...) themselves. Keeping them in the startup readiness set can block the provider's first sync until the readiness timeout expires. Removing them from readiness gating avoids that unnecessary startup delay while preserving readiness coverage for the resources that already participate in the startup lifecycle.

Testing

  • go test $(go list ./... | grep -v /e2e | grep -v /conformance)

Summary by CodeRabbit

  • Chores
    • Updated readiness monitoring to track Ingress, ApisixRoute, ApisixGlobalRule, ApisixTls, and ApisixConsumer resources; removed monitoring for ApisixPluginConfig and ApisixUpstream.
    • Enhanced resource availability detection to conditionally handle Ingress resources based on cluster configuration.
  • Tests
    • Added test coverage for readiness resource monitoring.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 8, 2026 09:04
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3ee3f1e0-ca68-40fa-99cb-9da074a029be

📥 Commits

Reviewing files that changed from the base of the PR and between e5a51c9 and bcd4679.

📒 Files selected for processing (2)
  • internal/manager/controllers.go
  • internal/manager/controllers_test.go

📝 Walkthrough

Walkthrough

This PR refactors v2 readiness resource registration to use a helper function instead of a hardcoded list. The new helper returns the set of resources (Ingress, ApisixRoute, ApisixGlobalRule, ApisixTls, ApisixConsumer) to register. The registration logic now conditionally skips Ingress when the API type is unavailable. Tests validate the helper's output.

Changes

Readiness Resource Registration Refactor

Layer / File(s) Summary
Readiness Resource Definition
internal/manager/controllers.go
New v2ReadinessResources() helper returns v2 client objects for readiness checks: Ingress, ApisixRoute, ApisixGlobalRule, ApisixTls, ApisixConsumer. Excludes ApisixPluginConfig and ApisixUpstream from previous hardcoded list.
GVK Registration with Conditional Ingress
internal/manager/controllers.go
registerV2ForReadinessGVK builds GVKs from v2ReadinessResources() and conditionally skips netv1.Ingress registration when the API type is not present in the cluster. Ingress-class-based filtering logic unchanged.
Test Coverage
internal/manager/controllers_test.go
TestV2ReadinessResources validates that the helper returns required resources (Ingress, ApisixRoute, ApisixGlobalRule, ApisixTls, ApisixConsumer) and excludes ApisixPluginConfig and ApisixUpstream.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • ronething
🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning E2E test required per custom check. PR provides only unit test (TestV2ReadinessResources). Lacks full startup readiness flow coverage: conditional filtering, Manager integration, Filter registration. Add E2E tests: startup readiness timing with excluded resources, Ingress conditional filtering, included resources gate startup, excluded resources don't delay startup.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: excluding status-only API v2 resources from readiness gating, which aligns with the primary objective of removing ApisixPluginConfig and ApisixUpstream from startup readiness checks.
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 vulnerabilities detected. PR safely modifies readiness GVK registration, excluding status-only resources. Logging only includes metadata without sensitive content.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch backport/pr-2745

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts API v2 startup readiness gating to avoid blocking initial sync on status-oriented resources by removing ApisixPluginConfig and ApisixUpstream from the v2 readiness GVK registration set, and adds a unit test intended to prevent regressions in the readiness resource list.

Changes:

  • Refactor v2 readiness registration to build the readiness GVK list from a centralized v2ReadinessResources() list.
  • Exclude ApisixPluginConfig and ApisixUpstream from v2 readiness gating.
  • Add a unit test covering inclusion/exclusion of v2 readiness resources.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/manager/controllers.go Moves v2 readiness GVK selection to v2ReadinessResources() and removes status-only v2 resources from readiness gating.
internal/manager/controllers_test.go Adds a unit test to validate the v2 readiness resource list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +36 to +55
want := []string{
types.GvkOf(&netv1.Ingress{}).String(),
types.GvkOf(&apiv2.ApisixRoute{}).String(),
types.GvkOf(&apiv2.ApisixGlobalRule{}).String(),
types.GvkOf(&apiv2.ApisixTls{}).String(),
types.GvkOf(&apiv2.ApisixConsumer{}).String(),
}
for _, gvk := range want {
if !seen[gvk] {
t.Fatalf("expected readiness resources to include %s", gvk)
}
}

notExpected := []string{
types.GvkOf(&apiv2.ApisixPluginConfig{}).String(),
types.GvkOf(&apiv2.ApisixUpstream{}).String(),
}
for _, gvk := range notExpected {
if seen[gvk] {
t.Fatalf("expected readiness resources to exclude %s", gvk)
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-08T09:18:36Z"
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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-08T09:19:06Z"
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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-08T09:42:42Z"
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:
    - GRPCExactMethodMatching
    - GRPCRouteHeaderMatching
    - GRPCRouteListenerHostnameMatching
    - GatewayModifyListeners
    result: failure
    statistics:
      Failed: 4
      Passed: 8
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests failed with 4 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.

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.

2 participants