Skip to content

Conversation

@AlinsRan
Copy link
Contributor

@AlinsRan AlinsRan commented Aug 21, 2025

Type of change:

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches
  • Documentation
  • Refactor
  • Chore
  • CI/CD or Tests

What this PR does / why we need it:

  1. Regardless of whether the ingressClass matches, we always update the resource status. This means that if two ingress controllers are deployed in the same cluster, both will attempt to update the status of the same resource. For example, an ApisixRoute may be managed by controller A, but if controller B also exists in the cluster, it may update the status of that resource to “failed,” leading to conflicts or confusion.
  • Reproduction steps: Only need to deploy 2 controllers and apply ApisixRoute.
  1. ingress-apisix/apisix-config and test-ic/test-gateway-proxy have different controllerName, but synchronization still occurs
2025-08-07T07:39:07Z        error        adc/adc.go:508        failed to execute adc command        {"error": "ADC execution error for test-ic/test-gateway-proxy: [ServerAddr: https://localhost:54272, Err: failed to sync resources: AggregateError, exit err: exit status 1]", "config": {"name":"api7-ic/api7-gateway-proxy","serverAddrs":["https://localhost:54272"],"tlsVerify":false}}
2025-08-07T07:39:09Z        error        adc/adc.go:388        failed to sync resources        {"name": "ingress-apisix/apisix-config", "error": "ADC execution errors: [ADC execution error for ingress-apisix/apisix-config: [ServerAddr: http://apisix-admin.ingress-apisix:9180, Err: failed to sync resources: AxiosError: Request failed with status code 404, exit err: exit status 1]]"}
2025-08-07T07:39:09Z        error        adc/adc.go:324        failed to sync 2 configs: test-ic/test-gateway-proxy, ingress-apisix/apisix-config
  1. Fixed that the controller name is always shared during concurrent testing.

  2. Fix 503 caused by using external services during testing.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@AlinsRan AlinsRan requested a review from ronething August 21, 2025 02:49
Copy link
Contributor

@ronething ronething left a comment

Choose a reason for hiding this comment

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

lgtm

@AlinsRan AlinsRan requested a review from Revolyssup August 21, 2025 04:00
@AlinsRan AlinsRan merged commit 227062d into apache:master Aug 21, 2025
31 of 33 checks passed
@AlinsRan AlinsRan deleted the fix/matched-controllername branch August 21, 2025 06:26
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