Fix Go duration parsing in kafkaCleaner test#2352
Fix Go duration parsing in kafkaCleaner test#2352delthas wants to merge 1 commit intodevelopment/2.14from
Conversation
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
SylvainSenechal
left a comment
There was a problem hiding this comment.
Hey I'm approving just some thoughts :
This new helper function is kinda ugly and will probably only be used once,
- I was thinking we could almost just drop the "KafkaCleanerInterval" parameters and just give that function 300sec to timeout.
- If you do keep the parseDurationSeconds, consider a few documentation example of what "duration: string" => result this function would process
More importantly : In that codebaase, we have Zenko.yaml, and it's got the kafkaCleaner.interval set to 1m, considering that all the tests we've got probably don't produce that many messages (maybe a few hundreds), and that the issue was probably situation where that kafka cleaner would start a few seconds before the wrong 60sec timeout instead of 600sec that we had, I would suggest lowering the value, somewhere between 15s and 30
Right. Testing with 15s, timeout of 5 minutes. Let's see how it goes. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following reviewers are expecting changes from the author, or must review again: |
ffe1221 to
b6e16fb
Compare
Documented. |
b6e16fb to
0f63d66
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
0f63d66 to
0285688
Compare
|
/approve |
Build failedThe build for commit did not succeed in branch bugfix/ZENKO-5218/fix-duration-parsing-kafkacleaner The following options are set: approve |
The KafkaCleanerInterval parameter is a Go duration string (e.g. "1m")
read from the Zenko CR spec.kafkaCleaner.interval. The test used
parseInt("1m") to parse it, which silently returned 1 instead of 60,
since parseInt stops at the first non-numeric character.
This made the test timeout after ~60s (1 * 6000ms * 10) instead of
~600s (60 * 6000ms * 10), giving the kafkacleaner only one cleaning
cycle to process all topics instead of ten. Under normal conditions
one cycle was often enough, but when operator reconciliation caused
topic recreation during the run, the kafkacleaner needed several
cycles to catch up, causing the test to fail with:
"Kafka cleaner did not clean the topics within the expected time"
Replace parseInt with a proper Go duration parser that handles
compound durations (e.g. "2h45m"), fractional values, and all
standard Go time units (ns, us, ms, s, m, h).
Issue: ZENKO-5218
0285688 to
1c24a01
Compare
Build failedThe build for commit did not succeed in branch bugfix/ZENKO-5218/fix-duration-parsing-kafkacleaner The following options are set: approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
Queue build failedThe corresponding build for the queue failed:
Remove the pull request from the queue
|
Summary
The
kafkaCleanerend-to-end test (features/zzz.kafkaCleaner.feature) readsKafkaCleanerIntervalfrom the Zenko CR'sspec.kafkaCleaner.intervalfield,which is a Go duration string (e.g.
"1m"). The test usedparseInt("1m")toconvert this to seconds, but
parseIntstops at the first non-numeric characterand silently returned
1instead of60.This caused the test to compute a timeout of ~60s (
1 × 6000ms × 10) insteadof the intended ~600s (
60 × 6000ms × 10), giving the kafkacleaner only onecleaning cycle to process all topics rather than ten. Under normal conditions one
cycle was often enough, but when operator reconciliation triggered topic
recreation mid-run, the kafkacleaner needed several cycles to catch up — and the
test failed with:
Fix
Replace
parseIntwith aparseDurationToSecondsutility that properly handlesGo-style duration strings — including compound durations (
"2h45m"), fractionalvalues (
"1.5s"), and all standard time units (ns,us/µs,ms,s,m,h).For the current
"1m"value, this correctly returns60seconds, restoring theintended 10-cycle timeout window.
Issue: ZENKO-5218