WIP kubeapiserver auditloganalyzer: spot handler panics in audit log #29127
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
| Output: "details in audit log", | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Add a success here so that we flake instead of fail to start. We can still search for flakes.
| for _, id := range panics.UnsortedList() { | ||
| ids = append(ids, string(id)) | ||
| } | ||
| failures := append(failures, fmt.Sprintf("failed auditIDs: %s", strings.Join(ids, ","))) |
There was a problem hiding this comment.
we need times for these to track them more effectively.
There was a problem hiding this comment.
we store audit-id, so later more info - like starting time - can be extracted. Do we want time to do more complicated alerting like "no more that n panics in x minutes"?
There was a problem hiding this comment.
Do we want time to do more complicated alerting like "no more that n panics in x minutes"?
I don't think these are panics. They may look like panics, but that's because the timeout handler prints stacks.
we store audit-id, so later more info - like starting time - can be extracted.
you store the entire auditEvent to pull the times out. It's just a lot more useful to see it on the spyglass page and compare it to "what's happening then" than to download an audit log and parse backwards.
vrutkovs
force-pushed
the
auditloganalyzer-handler-panics
branch
from
5115e08 to
341a9c5
Compare
|
Job Failure Risk Analysis for sha: 341a9c5
|
vrutkovs
force-pushed
the
auditloganalyzer-handler-panics
branch
3 times, most recently
from
56291b5 to
96386f9
Compare
|
Job Failure Risk Analysis for sha: 96386f9
|
openshift-merge-robot
added
the
needs-rebase
Don't let any useragent cause too many panics in apiserver
vrutkovs
force-pushed
the
auditloganalyzer-handler-panics
branch
from
96386f9 to
74b2b95
Compare
openshift-merge-robot
removed
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vrutkovs The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -367,6 +370,37 @@ func (w *auditLogAnalyzer) EvaluateTestsFromConstructedIntervals(ctx context.Con | |||
|
|
|||
| ret = append(ret, w.violationChecker.CreateJunits()...) | |||
|
|
|||
| for userAgent, userAgentPanics := range w.apiserverPanicChecker.panicEventsPerUserAgent.panicEvents { | |||
| testName := fmt.Sprintf("user %s must not produce too many apiserver handler panics", userAgent) | |||
There was a problem hiding this comment.
This is generating random test names, infinitely increasing the tests in sippy even as a presubmit. 150 tests so far. We'll clean those up but this needs to be a megatest, can't do a test per uncontrolled user inputs.
There was a problem hiding this comment.
ex name | user ip-10-0-21-189/ovnkube@038aea1608e0 (linux/amd64) kubernetes/v0.31.1 must not produce too many apiserver handler panics
There was a problem hiding this comment.
Right, this may not have an authenticated user.
Should we use "too many apiserver handler panics produced" as testname and move useragent to the error message?
Perhaps, instead we could add username if available and use "unknown" otherwise? The usernames may be infinite too though
There was a problem hiding this comment.
It does seem dangerous even if just authed users, presumably tests could be making some with random names? Typically we would write a whitelist for known users you're particularly interested in broken into their own tests, plus a fallback catch all single test, or just the fallback single test.
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. Instructions 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-sigs/prow repository. |
|
@vrutkovs: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions 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-sigs/prow repository. I understand the commands that are listed here. |
Don't let any useragent cause too many panics in apiserver