📝 Description
The E2E helper MaxConcurrency (test/pkg/wait/concurrency.go:100) computes,
from recorded start and end times, how many PipelineRuns were running at the
same moment. AssertMaxConcurrency uses it to check that a concurrency_limit
was actually respected. Its doc comment promises the computation "cannot hide
an overshoot".
It can. For a run that is still going, the helper substitutes the latest
timestamp seen anywhere as the end time. When the still-running run is the one
that started last, that substitute equals its own start time, the zero-length
guard discards the interval, and the run vanishes from the count.
The run that started last while others were still going is precisely the run a
broken queue admitted over the limit. Two runs clearly running side by side
count as one:
- run A started at 10:00, still running
- run B started at 10:05, still running
- reported peak: 1
A unit test confirms this. Nothing is hidden in CI today, because every
current caller waits for all runs to finish before calling the helper. The
helper is exported, though, and its promise does not hold.
Found during a post-merge review of #2890.
🛠️ Suggested fix
Give unfinished runs an end time strictly after every recorded start, for
example the latest timestamp plus a nanosecond, or pass in the time the
snapshot was taken. Leave the zero-length rule for finished runs as it is:
Kubernetes stores these timestamps with one-second precision, so two runs
inside the same second cannot be proven to overlap, and counting them would
turn a missed detection into a false alarm.
🧪 Testing Strategy
📝 Description
The E2E helper
MaxConcurrency(test/pkg/wait/concurrency.go:100) computes,from recorded start and end times, how many PipelineRuns were running at the
same moment.
AssertMaxConcurrencyuses it to check that aconcurrency_limitwas actually respected. Its doc comment promises the computation "cannot hide
an overshoot".
It can. For a run that is still going, the helper substitutes the latest
timestamp seen anywhere as the end time. When the still-running run is the one
that started last, that substitute equals its own start time, the zero-length
guard discards the interval, and the run vanishes from the count.
The run that started last while others were still going is precisely the run a
broken queue admitted over the limit. Two runs clearly running side by side
count as one:
A unit test confirms this. Nothing is hidden in CI today, because every
current caller waits for all runs to finish before calling the helper. The
helper is exported, though, and its promise does not hold.
Found during a post-merge review of #2890.
🛠️ Suggested fix
Give unfinished runs an end time strictly after every recorded start, for
example the latest timestamp plus a nanosecond, or pass in the time the
snapshot was taken. Leave the zero-length rule for finished runs as it is:
Kubernetes stores these timestamps with one-second precision, so two runs
inside the same second cannot be proven to overlap, and counting them would
turn a missed detection into a false alarm.
🧪 Testing Strategy