-
Notifications
You must be signed in to change notification settings - Fork 2
feat: support apisixroute resolveGranularity #165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2025-06-17T03:44:07Z"
gatewayAPIChannel: standard
gatewayAPIVersion: v1.2.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:
- HTTPRouteCrossNamespace
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. |
There was a problem hiding this 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 introduces support for ApisixRoute resolveGranularity, allowing the controller and translator to distinguish between "service" and "endpoint" granularity for backend resolution. Key changes include:
- Modifications to end-to-end tests to verify both a service-not-found case and proper resolveGranularity behavior.
- Updates to the translator to add distinct functions for handling backend resolution for service and endpoint granularity.
- Adjustments to the ApisixRoute controller to log errors and continue processing in place of returning errors for certain service-related issues.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/e2e/framework/assertion.go | Removed a type check on the object in APIv2MustHaveCondition to align with the new resolveGranularity support. |
| test/e2e/apisix/route.go | Added new test cases for service-not-found and proper resolveGranularity handling, and updated test imports. |
| internal/provider/adc/translator/apisixroute.go | Introduced two new functions for translating backends based on the resolveGranularity field and adjusted utils usage. |
| internal/controller/apisixroute_controller.go | Modified error-handling to log errors and continue, rather than immediately returning errors for service issues. |
Comments suppressed due to low confidence (2)
internal/controller/apisixroute_controller.go:224
- Consider reviewing whether logging the error and continuing when a service does not have a ClusterIP is the desired behavior, as this might hide configuration issues that should be addressed.
if backend.ResolveGranularity == "service" && service.Spec.ClusterIP == "" {
internal/controller/apisixroute_controller.go:213
- Verify that using client.IgnoreNotFound to skip processing for a missing service is in line with the intended behavior and will not mask underlying issues that could be critical during runtime.
if err := client.IgnoreNotFound(err); err == nil {
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2025-06-17T03:47:36Z"
gatewayAPIChannel: standard
gatewayAPIVersion: v1.2.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: partial
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 0
Passed: 32
Skipped: 1
name: GATEWAY-HTTP
summary: Core tests partially succeeded with 1 test skips. |
| var ( | ||
| upNodes adc.UpstreamNodes | ||
| ) | ||
| if backend.ResolveGranularity == "service" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does "service" consider defining a constant?
Type of change:
What this PR does / why we need it:
Support ApisixRoute resolveGranularity.
Pre-submission checklist: