Fix: resource-watcher crashes, crossed events, dropped creates and no-op updates

BR
1mo ago
+349-56

Description of your changes

Four resource-watcher bugs, one commit each, each with a test that fails without its fix.

CommitBugEffect
Handle a delete the watcher only saw on a relistA missed delete arrives as a cache.DeletedFinalStateUnknown tombstone, sometimes with no object in it; the handler type-asserted it to metav1.Objectkube-trigger panics and restarts
Give each trigger only the events it asked forTriggers watching the same resources share one watcher, and their events lists were merged for every handlerA 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 timeThe "skip creates older than start" time was a package global that every watcher's Run overwroteStarting a second watcher (another cluster or kind) made earlier ones drop real creates; also a data race
Skip updates that changed nothingA relist reports every cached object as an updateEvery trigger's filter and action ran for every object after a long disconnect

Changes

  • utils.GetObjectMetaData unwraps 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, so InformerEvent.EventObj is now a metav1.Object.
  • The three informer handlers share one enqueue (moved into resourceEventHandler so tests can call it), replacing a variable they all mutated.
  • Config.Merge keeps no events (all) when either side has none; each trigger's handler is wrapped in controller.ForEvents with its own event types.
  • Controller.startTime replaces the global.
  • UpdateFunc skips an update whose resourceVersion is 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 reviewable to ensure this PR is ready for review. (Ran make lint, make checklicense and go mod tidy; not generate, as the local controller-gen differs from the pinned v0.9.0.)
  • Added backport release-x.y labels 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

  • GetObjectMetaData unwraps tombstones and returns (metav1.Object, bool); events without an object are logged and skipped, and InformerEvent.EventObj is now metav1.Object.
  • The three informer handlers share one enqueue in resourceEventHandler.
  • Config.Merge keeps no events (all) when either side has none, and each trigger's handler is wrapped in ForEvents with its own event types.
  • Controller.startTime replaces the global start time.
  • Updates are skipped only when the old and new resourceVersion are 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.