Figure out if we need knative/eventing#4296

[grantr]

knative/eventing#4296 changes the way trigger dependencies are tracked by switching from the kresource duck to the source duck. Do we need this change too?

[Ectelion]

The Eventing's construct for tracking trigger dependencies was inherited by the trigger reconciler in knative-gcp repo. Thus, we may see the same mismatch between the informer and lister in pkg/reconciler/trigger/trigger, pkg/reconciler/trigger/controller and related components.

On the other hand, in case of Knative-GCP, TestGrid results had no indication of flakiness for the relevant TestTriggerDependencyAnnotation test (at least for the available last few weeks). Note that in the PR addressing knative/eventing#4296, there appear to be two distinct fixes: one for the discussed informer/lister issue and the other had to do with a broker->trigger typo (see checkDependencyAnnotation function). It is unclear whether the test failures in the Eventing repo rather had to do with the latter problem, which is non-existent in the knative-gcp's trigger implementation. This may also have to do with the different test environments.

Regardless of test flakiness results, addressing the informer/lister mismatch, as suggested in knative/eventing/pull/4296 and keeping the dependency tracking logic consistent with the Eventing implementation seems to make sense. Relevant PR is sent.

[Ectelion]

Actually, the above-mentioned issue with broker->trigger typo was not the source of flakiness for that particular test, because trigger and broker namespaces should be the same anyways.

Also, in case you wonder why the previous version still worked. There are two pieces here: Informer and Lister. When it comes to listing, the difference is in KSources and Sources ducks, which are very similar/substitutable ones, so, listing should work consistently in both cases. Informer is more tricky and my understanding is that it did work before because the Informer events in both cases are inter-related, i.e. a source change and a condition change tend to co-occur with very close timing, but are not quite guaranteed to be always consistent with a different lister.

[grantr]

Thanks @Ectelion for investigating and fixing this!