Fix: resource-watcher crashes, crossed events, dropped creates and no-op updates
Description of your changes
Four resource-watcher bugs, one commit each, each with a test that fails without its fix.
| Commit | Bug | Effect |
|---|---|---|
| Handle a delete the watcher only saw on a relist | A missed delete arrives as a cache.DeletedFinalStateUnknown tombstone, sometimes with no object in it; the handler type-asserted it to metav1.Object | kube-trigger panics and restarts |
| Give each trigger only the events it asked for | Triggers watching the same resources share one watcher, and their events lists were merged for every handler | A trigger with events: [create] also fired on updates when another trigger on the same kind watched [update]. Worse, a trigger with no events (all) merged with [update] was narrowed to updates only |
| Keep each resource watcher's own start time | The "skip creates older than start" time was a package global that every watcher's Run overwrote | Starting a second watcher (another cluster or kind) made earlier ones drop real creates; also a data race |
| Skip updates that changed nothing | A relist reports every cached object as an update | Every trigger's filter and action ran for every object after a long disconnect |
Changes
utils.GetObjectMetaDataunwraps tombstones and returns(metav1.Object, bool); an event with no object is logged and skipped. It is the one check every informer event goes through, soInformerEvent.EventObjis now ametav1.Object.- The three informer handlers share one
enqueue(moved intoresourceEventHandlerso tests can call it), replacing a variable they all mutated. Config.Mergekeeps no events (all) when either side has none; each trigger's handler is wrapped incontroller.ForEventswith its own event types.Controller.startTimereplaces the global.UpdateFuncskips an update whoseresourceVersionis set and equals the cached one.
I have:
- Read and followed KubeVela's pull request process.
- All commits are signed off (DCO).
- Related Docs updated properly. Not needed: these restore the documented behaviour.
- 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 commit has a test that fails without its fix: tombstones with and without an object, per-trigger events and the merge of event lists (including overlapping ones), a second watcher's start not dropping the first's creates, and updates with an unchanged, changed or missing resourceVersion.
go test ./pkg/... passes, and the changed packages pass under -race. Four existing pkg/workqueue tests fail under -race on main as well; they are untouched here.
The K8sResourceWatcher.Init line that applies ForEvents has no direct test, since that package's init() needs a kubeconfig; Merge and ForEvents are tested on their own. Not yet run against a live cluster.
Special notes for your reviewer
- #63 builds on this PR.
Summary by cubic
Fixes four resource-watcher bugs: crashes on tombstoned deletes, triggers firing on events they didn't ask for, a shared start time dropping real creates, and relist no-op updates.
Changes
GetObjectMetaDataunwraps tombstones and returns(metav1.Object, bool); events without an object are logged and skipped, andInformerEvent.EventObjis nowmetav1.Object.- The three informer handlers share one
enqueueinresourceEventHandler. Config.Mergekeeps no events (all) when either side has none, and each trigger's handler is wrapped inForEventswith its own event types.Controller.startTimereplaces the global start time.- Updates are skipped only when the old and new
resourceVersionare equal and non-empty; an empty version says nothing.
Testing
go test ./pkg/... passes, and the changed packages pass under -race; four existing pkg/workqueue tests fail under -race on main as well and are untouched. Not yet run against a live cluster.