Skip to content

chore: add resource context to status error messages and logs#2735

Open
BigOrangeQWQ wants to merge 4 commits intoapache:masterfrom
BigOrangeQWQ:chore/enrich-status-error-messages
Open

chore: add resource context to status error messages and logs#2735
BigOrangeQWQ wants to merge 4 commits intoapache:masterfrom
BigOrangeQWQ:chore/enrich-status-error-messages

Conversation

@BigOrangeQWQ
Copy link
Copy Markdown

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:

Fixes #2714
When APISIX sync fails, the error messages in resource status lack resource context (kind/namespace/name), making it difficult to quickly identify which resource caused the problem.

This PR enriches error messages and logs with resource information to make troubleshooting easier.

Before:

conditions:
  - lastTransitionTime: "2026-02-03T18:36:37Z"
    message: 'ServerAddr: http://10.244.0.6:9180, Err: invalid routes at index 1, err: unknown plugin [non-existent-plugin]'
    observedGeneration: 1
    reason: SyncFailed
    status: "False"
    type: Accepted

After:

conditions:
  - lastTransitionTime: "2026-03-25T11:46:01Z"
    message: 'ServerAddr: http://10.42.0.9:9180, Err: invalid routes at index 0, err:
      unknown plugin [non-existent-plugin] (resource: ApisixRoute/hello-world/hello-world-2)'
    observedGeneration: 1
    reason: SyncFailed
    status: "False"
    type: Accepted

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

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 improves troubleshooting for APISIX sync failures by enriching status condition messages and some error logs with Kubernetes resource context (kind/namespace/name), making it easier to correlate a failure back to the originating object.

Changes:

  • Add a helper to append (resource: Kind/namespace/name) to sync failure error messages when resource labels are available.
  • Update status failure handling to use the enriched message for both “empty” and “detailed” failure paths.
  • Add unit tests for the message builder and status update map keying behavior.

Reviewed changes

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

File Description
internal/provider/apisix/status.go Builds enriched error messages with resource context and adds more context to failure logs during status updates.
internal/provider/apisix/status_test.go Adds unit tests covering the new error-message formatting helper and status map updates.

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

Comment thread internal/provider/apisix/status.go
@BigOrangeQWQ
Copy link
Copy Markdown
Author

BigOrangeQWQ commented Mar 26, 2026

logger.go:67: Configuring Kubernetes client using config file /home/runner/.kube/config with context 
  << Captured StdOut/StdErr Output
------------------------------

Summarizing 1 Failure:
  [FAIL] Test TLSRoute TLSRoute Base [It] Basic [networking.k8s.io, tlsroute]
  /home/runner/work/apisix-ingress-controller/apisix-ingress-controller/test/e2e/gatewayapi/tlsroute.go:91

Ran 79 of 195 Specs in 1566.089 seconds
FAIL! -- 78 Passed | 1 Failed | 1 Pending | 115 Skipped

The test failure occurred in tlsroute.go. This appears to be a pre-existing bug unrelated to the changes in this PR.
I'd like to create a separate PR to address this issue if that's okay.


It looks like this issue might be related to the changes in this PR. I might have misjudged earlier. I'm investigating it now.

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.

feat: As a sysadmin, I want better feedback on invalid routes, so that i can troubleshoot easier

3 participants