fix(api,watcher): cascade delete Results on namespace deletion - #1411
fix(api,watcher): cascade delete Results on namespace deletion #1411pratap0007 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…p Loki/Splunk HTTP errors to proper gRPC codes When a namespace is deleted, Results API records persisted in Postgres but log retrieval failed with a generic 500 Internal Error. The root cause was twofold: 1. The watcher had no Namespace informer, so Results/Records were never cleaned up when a namespace was deleted, leaving orphaned data. 2. The log plugin mapped ALL non-200 HTTP responses from Loki/Splunk to codes.Internal with a generic error message. When Loki returned 403 Forbidden for a deleted namespace tenant, this surfaced as an unhelpful "Error fetching log data (code: 500)" to the user. This commit addresses both issues: **Namespace cascade deletion (Fix 1):** - Add a new Namespace reconciler (pkg/watcher/reconciler/namespace/) that watches for namespace deletion events via the Kubernetes informer - On namespace deletion or DeletionTimestamp being set, the reconciler paginates through ListResults and calls DeleteResult for each result in that namespace. DB-level cascade (OnDelete:CASCADE) handles Records - Register the new controller in the watcher's main.go **HTTP-to-gRPC error mapping (Fix 2):** - Add httpStatusToGRPCCode() helper mapping HTTP 400->InvalidArgument, 401->Unauthenticated, 403->PermissionDenied, 404->NotFound, 429->ResourceExhausted, and default->Internal - Apply mapping to Loki and both Splunk error paths, including the HTTP status code in the error message for debuggability - Fix response body leak in getLokiLogs (missing defer resp.Body.Close) Resolves: SRVKP-12883 /kind bug Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c4db20d to
37064cf
Compare
khrm
left a comment
There was a problem hiding this comment.
Tbh, it makes sense to persist data for deleted namespace. It's an issue from Loki side that it's not persisting data.
khrm
left a comment
There was a problem hiding this comment.
Are we sure that this issue exist with Splunk. Also, does this issue exist with only OpenShift Loki or normal Loki also.
I agree. We should not simply purge the data for deleted namespace. The namespace could be deleted by mistake and easily re-created, but by that time the archived data would be gone. Furthermore, I think this should not be the watcher responsibility and I don't like this extra delete permission added to it. Instead that could be made responsibility of the retention-policy-agent (it already has this delete permission and that's its primary job) which can be enabled by the user with some flag e.g. Providing proper error message is important though. |
It has an issue with Loki, and I haven't checked it with Splunk |
|
@pratap0007: PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
fix(api,watcher): cascade delete Results on namespace deletion and map Loki/Splunk HTTP errors to proper gRPC codes
When a namespace is deleted, Results API records persisted in Postgres but log retrieval failed with a generic 500 Internal Error. The root cause was twofold:
The watcher had no Namespace informer, so Results/Records were never cleaned up when a namespace was deleted, leaving orphaned data.
The log plugin mapped ALL non-200 HTTP responses from Loki/Splunk to codes.Internal with a generic error message. When Loki returned 403 Forbidden for a deleted namespace tenant, this surfaced as an unhelpful "Error fetching log data (code: 500)" to the user.
This commit addresses both issues:
Namespace cascade deletion (Fix 1):
HTTP-to-gRPC error mapping (Fix 2):
Resolves: SRVKP-12883
/kind bug
Changes
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you review them:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes