Repository: knative/pkg
Pull request #2220 (2021-08-20 22:05:29)

Have the addressable resolver take a tracker.
I noticed in a few places downstream that reconcilers were creating both trackers and Addressable resolvers, which seems superfluous.  As part of examining the way we use the tracker, I'm experimenting with changing this to just take a tracker.

/kind cleanup

**Release Note**

```release-note
resolver.NewURIResolver now takes a tracker.Interface instead of a callback.
```

**Docs**

```docs

```


knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2220#" title="Author self-approved">mattmoor</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[resolver/OWNERS](https://github.com/knative/pkg/blob/main/resolver/OWNERS)~~ [mattmoor]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
mattmoor: PSA: I'm opening this to survey what breaks, I don't plan to check this in as-is.
mattmoor: /assign @n3wscott 

knative-prow-robot: @mattmoor: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 7f7bbe29e0909a9f65d2ed6b07b6d909c672e659 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2220/pull-knative-pkg-unit-tests/1428869173500448768) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/2220). [Your PR dashboard](https://gubernator.knative.dev/pr/mattmoor).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
mattmoor: /retest
n3wscott: /lgtm
/approve

Patch:
diff --git a/resolver/addressable_resolver.go b/resolver/addressable_resolver.go
index efd878f14..01443cf6e 100644
--- a/resolver/addressable_resolver.go
+++ b/resolver/addressable_resolver.go
@@ -44,12 +44,17 @@ type URIResolver struct {
 	listerFactory func(schema.GroupVersionResource) (cache.GenericLister, error)
 }
 
-// NewURIResolver constructs a new URIResolver with context and a callback
-// for a given listableType (Listable) passed to the URIResolver's tracker.
+// NewURIResolver constructs a new URIResolver with context and a tracker.
+// Deprecated: use NewURIResolverFromTracker instead.
 func NewURIResolver(ctx context.Context, callback func(types.NamespacedName)) *URIResolver {
-	ret := &URIResolver{}
+	return NewURIResolverFromTracker(ctx, tracker.New(callback, controller.GetTrackerLease(ctx)))
+}
 
-	ret.tracker = tracker.New(callback, controller.GetTrackerLease(ctx))
+// NewURIResolverFromTracker constructs a new URIResolver with context and a tracker.
+func NewURIResolverFromTracker(ctx context.Context, t tracker.Interface) *URIResolver {
+	ret := &URIResolver{
+		tracker: t,
+	}
 
 	informerFactory := &pkgapisduck.CachedInformerFactory{
 		Delegate: &pkgapisduck.EnqueueInformerFactory{


Test patch:
diff --git a/resolver/addressable_resolver_test.go b/resolver/addressable_resolver_test.go
index 8cc88ec32..83b8fa6e8 100644
--- a/resolver/addressable_resolver_test.go
+++ b/resolver/addressable_resolver_test.go
@@ -35,6 +35,7 @@ import (
 	"knative.dev/pkg/client/injection/ducks/duck/v1/addressable"
 	fakedynamicclient "knative.dev/pkg/injection/clients/dynamicclient/fake"
 	"knative.dev/pkg/resolver"
+	"knative.dev/pkg/tracker"
 )
 
 const (
@@ -353,7 +354,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 		t.Run(n, func(t *testing.T) {
 			ctx, _ := fakedynamicclient.With(context.Background(), scheme.Scheme, tc.objects...)
 			ctx = addressable.WithDuck(ctx)
-			r := resolver.NewURIResolver(ctx, func(types.NamespacedName) {})
+			r := resolver.NewURIResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0))
 
 			// Run it twice since this should be idempotent. URI Resolver should
 			// not modify the cache's copy.
@@ -538,7 +539,7 @@ func TestGetURIDestinationV1(t *testing.T) {
 		t.Run(n, func(t *testing.T) {
 			ctx, _ := fakedynamicclient.With(context.Background(), scheme.Scheme, tc.objects...)
 			ctx = addressable.WithDuck(ctx)
-			r := resolver.NewURIResolver(ctx, func(types.NamespacedName) {})
+			r := resolver.NewURIResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0))
 
 			// Run it twice since this should be idempotent. URI Resolver should
 			// not modify the cache's copy.
@@ -580,7 +581,7 @@ func TestURIFromObjectReferenceErrors(t *testing.T) {
 		t.Run(n, func(t *testing.T) {
 			ctx, _ := fakedynamicclient.With(context.Background(), scheme.Scheme, tc.objects...)
 			ctx = addressable.WithDuck(ctx)
-			r := resolver.NewURIResolver(ctx, func(types.NamespacedName) {})
+			r := resolver.NewURIResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0))
 
 			// Run it twice since this should be idempotent. URI Resolver should
 			// not modify the cache's copy.