Fix: run same-ID jobs one at a time, in order, without dropping any
Description of your changes
Problem
A trigger that watches more than one event type (events: [create, update, delete]) looks like it only acts on one of them.
The events themselves all arrive. Every action job from one trigger shares a job ID (the hash of its action spec), and action jobs disallow concurrency. But the executor checked "is this ID running?" and then "mark it running" as two separate steps, so parallel workers all got past the check and the jobs ran at the same time. bump-application-revision reads publishVersion and patches it to N+1, so overlapping runs overwrite each other and only one bump lands.
Closing that race on its own isn't enough. A job that found its ID busy was requeued through the failure rate limiter, so it:
- waited on exponential backoff (2s, 4s, 8s... at the default
--retry-delay), even with--action-retryoff, - used up its retry budget while waiting,
- was dropped for good after
--max-retryattempts.
Fix
Each ID gets a waiting line, formed in AddJob:
| Before | After | |
|---|---|---|
| Job added while its ID is held | Queued, then requeued with failure backoff when picked up | Joins the back of that ID's line |
| Job finishes | Clears the running marker | Queues the next job in its ID's line |
| Same-ID jobs overlap | Yes, the check was racy | Never |
| Order | Arbitrary | Order added |
| Dropped for waiting | After --max-retry | Never |
| Failed job retrying | Later events could overtake it | Keeps its ID, so the line waits for it |
| Jobs that allow concurrency | Set and cleared the shared marker | Bypass the lines |
The queue size limit counts waiting jobs as well.
On shutdown, a job cancelled mid-run releases its ID, and once workers stop, the executor logs how many queued and waiting jobs never started, then resets the lines.
I have:
- Read and followed KubeVela's pull request process.
- All commits are signed off (DCO).
- Related Docs updated properly. Not needed: no user-facing configuration changes.
- Run
make reviewableto ensure this PR is ready for review. (Ranmake lint,make checklicenseandgo mod tidy; notgenerate, as the localcontroller-gendiffers from the pinned v0.9.0.) - Added
backport release-x.ylabels to auto-backport this PR if necessary. Not needed: kube-trigger has no release branches.
How has this code been tested
Each new test fails on main and passes with this change (-race -count=10):
TestSameJobNeverRunsConcurrently: three read-modify-write jobs sharing an ID must each apply.TestSameJobBurstRunsEveryJobInOrder: six same-ID jobs withMaxJobRetries: 0all run, in order, never overlapping.TestConcurrentJobDoesNotReleaseSharedID: a concurrency-allowed job sharing an ID doesn't free another job's claim.TestRetryKeepsItsPlaceInLine: a job that fails twice still runs before the same-ID job added after it.TestShutdownReleasesAndClearsLines: a job cancelled by shutdown releases its ID, and nothing is left waiting.
TestSameJobRequeuing is removed: it asserted contention retries, which no longer exist, and it discarded the error from its only check.
Not verified against a live cluster; this repo has no e2e suite yet.
Special notes for your reviewer
- Nothing special: a self-contained executor change.
Summary by cubic
Fixes the executor so triggers watching multiple event types no longer run their jobs concurrently, reorder them, or drop them for waiting on a busy ID.
- Replaces the racy is-running check plus mark with a waiting line per ID formed in
AddJob: the first job holds the ID, later same-ID jobs queue behind it, and releasing the ID starts the next in line. - A failed job keeps its ID through retries so later events can't overtake it; jobs that allow concurrency bypass the lines, and the queue size limit counts waiting jobs.
- Shutdown now releases the ID of a cancelled running job and logs and clears the jobs that never started, queued or waiting. The counts are approximate after a shutdown timeout, and the workqueue is not drained since it shuts down with the process.
Removes TestSameJobRequeuing, which asserted contention retries that no longer exist, and adds tests for the new behavior, including one asserting cancelled jobs release their ID.