Repository: knative/pkg
Pull request #2228 (2021-08-24 00:30:52)

Drop several deprecated APIs
# Changes

/kind cleanup

**Release Note**

```release-note
Drop deprecated addressable resolver constructor.
Drop deprecated controller.New* constructors.
```

**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/2228#" 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: WIP because I'm using this to find out what needs fixing downstream 😉 
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/2228?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) Report
> Merging [#2228](https://codecov.io/gh/knative/pkg/pull/2228?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (0749f1a) into [main](https://codecov.io/gh/knative/pkg/commit/12d915ede293cc22c2e6359ac12e739f0ab83681?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (12d915e) will **decrease** coverage by `0.01%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/2228/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)](https://codecov.io/gh/knative/pkg/pull/2228?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)

```diff
@@            Coverage Diff             @@
##             main    #2228      +/-   ##
==========================================
- Coverage   63.89%   63.88%   -0.02%     
==========================================
  Files         220      220              
  Lines        9536     9535       -1     
==========================================
- Hits         6093     6091       -2     
  Misses       3169     3169              
- Partials      274      275       +1     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/2228?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) | Coverage Δ | |
|---|---|---|
| [resolver/addressable\_resolver.go](https://codecov.io/gh/knative/pkg/pull/2228/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cmVzb2x2ZXIvYWRkcmVzc2FibGVfcmVzb2x2ZXIuZ28=) | `95.34% <ø> (+1.09%)` | :arrow_up: |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/2228/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `90.32% <0.00%> (-1.08%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/2228?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/2228?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Last update [12d915e...0749f1a](https://codecov.io/gh/knative/pkg/pull/2228?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2228#" 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:

- ~~[controller/OWNERS](https://github.com/knative/pkg/blob/main/controller/OWNERS)~~ [mattmoor]
- ~~[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":[]} -->
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 | 61fa1b9f0457d82c5616886c95682df64059a637 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2228/pull-knative-pkg-unit-tests/1430226648661233664) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/2228). [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 -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2228#" title="Author self-approved">mattmoor</a>*, *<a href="https://github.com/knative/pkg/pull/2228#pullrequestreview-737940888" title="LGTM">matzew</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:

- ~~[controller/OWNERS](https://github.com/knative/pkg/blob/main/controller/OWNERS)~~ [mattmoor]
- ~~[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":[]} -->
knative-prow-robot: New changes are detected. LGTM label has been removed.
mattmoor: I touched and repushed to retrigger validation, and (so far) it looks like what remains is that `eventing-kafka-broker` and `operator` both vendor stale codegen from eventing, which reference the old `controller.NewImpl` methods.  Once these pick up the new codegen (assuming nothing else pops out) we should be good to go. 🤞 
matzew: broker: will check !

On Wed 25. Aug 2021 at 17:45, Matt Moore ***@***.***> wrote:

> Looks like the eventing-kafka-broker update is blocked here:
> knative-sandbox/eventing-kafka-broker#1163
> <https://github.com/knative-sandbox/eventing-kafka-broker/pull/1163>
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub
> <https://github.com/knative/pkg/pull/2228#issuecomment-905631700>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AABGPTV6AON3XDMF4ZD7DQLT6UFX5ANCNFSM5CVW3MWA>
> .
>
-- 
Sent from Gmail Mobile

pierDipi: Fix for eventing-kafka-broker https://github.com/knative-sandbox/eventing-kafka-broker/pull/1171
mattmoor: Alright, all of the downstream tests, including Tekton are passing with this, so I think I'm going to pull the trigger on it!  :party-matt:
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2228#" title="Author self-approved">mattmoor</a>*, *<a href="https://github.com/knative/pkg/pull/2228#pullrequestreview-737940888" title="LGTM">matzew</a>*, *<a href="https://github.com/knative/pkg/pull/2228#pullrequestreview-742039471" title="Approved">n3wscott</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:

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

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":[]} -->

Patch:
diff --git a/controller/controller.go b/controller/controller.go
index dc7881ee7..6d8035fe9 100644
--- a/controller/controller.go
+++ b/controller/controller.go
@@ -233,19 +233,6 @@ type ControllerOptions struct { //nolint // for backcompat.
 	Concurrency   int
 }
 
-// NewImpl instantiates an instance of our controller that will feed work to the
-// provided Reconciler as it is enqueued.
-// Deprecated: use NewImplFull.
-func NewImpl(r Reconciler, logger *zap.SugaredLogger, workQueueName string) *Impl {
-	return NewImplFull(r, ControllerOptions{WorkQueueName: workQueueName, Logger: logger})
-}
-
-// NewImplFull accepts the full set of options available to all controllers.
-// Deprecated: use NewContext instead.
-func NewImplFull(r Reconciler, options ControllerOptions) *Impl {
-	return NewContext(context.TODO(), r, options)
-}
-
 // NewContext instantiates an instance of our controller that will feed work to the
 // provided Reconciler as it is enqueued.
 func NewContext(ctx context.Context, r Reconciler, options ControllerOptions) *Impl {
diff --git a/resolver/addressable_resolver.go b/resolver/addressable_resolver.go
index 7ff359080..0e50a2578 100644
--- a/resolver/addressable_resolver.go
+++ b/resolver/addressable_resolver.go
@@ -22,7 +22,6 @@ import (
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/cache"
 	"knative.dev/pkg/client/injection/ducks/duck/v1/addressable"
 	"knative.dev/pkg/controller"
@@ -49,12 +48,6 @@ type URIResolver struct {
 	resolvers     []RefResolverFunc
 }
 
-// NewURIResolver constructs a new URIResolver with context, a tracker and an optional list of custom resolvers.
-// Deprecated: use NewURIResolverFromTracker instead.
-func NewURIResolver(ctx context.Context, callback func(types.NamespacedName), resolvers ...RefResolverFunc) *URIResolver {
-	return NewURIResolverFromTracker(ctx, tracker.New(callback, controller.GetTrackerLease(ctx)), resolvers...)
-}
-
 // NewURIResolverFromTracker constructs a new URIResolver with context, a tracker and an optional list of custom resolvers.
 func NewURIResolverFromTracker(ctx context.Context, t tracker.Interface, resolvers ...RefResolverFunc) *URIResolver {
 	ret := &URIResolver{


Test patch:
diff --git a/controller/controller_test.go b/controller/controller_test.go
index 3b46d30af..2e9fb45b3 100644
--- a/controller/controller_test.go
+++ b/controller/controller_test.go
@@ -715,7 +715,7 @@ func TestEnqueue(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			var rl workqueue.RateLimiter = testRateLimiter{t, 100 * time.Millisecond}
-			impl := NewImplFull(&nopReconciler{}, ControllerOptions{WorkQueueName: "Testing", Logger: TestLogger(t), RateLimiter: rl})
+			impl := NewContext(context.TODO(), &nopReconciler{}, ControllerOptions{WorkQueueName: "Testing", Logger: TestLogger(t), RateLimiter: rl})
 			test.work(impl)
 
 			impl.WorkQueue().ShutDown()
@@ -751,7 +751,7 @@ func pollQ(q workqueue.RateLimitingInterface, sig chan int) func() (bool, error)
 }
 
 func TestEnqueueAfter(t *testing.T) {
-	impl := NewImplFull(&nopReconciler{}, ControllerOptions{
+	impl := NewContext(context.TODO(), &nopReconciler{}, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      &FakeStatsReporter{},
@@ -818,7 +818,7 @@ func TestEnqueueAfter(t *testing.T) {
 }
 
 func TestEnqueueKeyAfter(t *testing.T) {
-	impl := NewImplFull(&nopReconciler{}, ControllerOptions{
+	impl := NewContext(context.TODO(), &nopReconciler{}, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      &FakeStatsReporter{},
@@ -880,7 +880,7 @@ func (cr *CountingReconciler) Reconcile(context.Context, string) error {
 
 func TestStartAndShutdown(t *testing.T) {
 	r := &CountingReconciler{}
-	impl := NewImplFull(&nopReconciler{}, ControllerOptions{
+	impl := NewContext(context.TODO(), &nopReconciler{}, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      &FakeStatsReporter{},
@@ -950,7 +950,7 @@ func TestStartAndShutdownWithLeaderAwareNoElection(t *testing.T) {
 			},
 		},
 	}
-	impl := NewImplFull(r, ControllerOptions{
+	impl := NewContext(context.TODO(), r, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      &FakeStatsReporter{},
@@ -1022,7 +1022,7 @@ func TestStartAndShutdownWithLeaderAwareWithLostElection(t *testing.T) {
 		},
 	)
 
-	impl := NewImplFull(&nopReconciler{}, ControllerOptions{
+	impl := NewContext(context.TODO(), &nopReconciler{}, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      &FakeStatsReporter{},
@@ -1066,7 +1066,7 @@ func TestStartAndShutdownWithLeaderAwareWithLostElection(t *testing.T) {
 func TestStartAndShutdownWithWork(t *testing.T) {
 	r := &CountingReconciler{}
 	reporter := &FakeStatsReporter{}
-	impl := NewImplFull(r, ControllerOptions{
+	impl := NewContext(context.TODO(), r, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      reporter,
@@ -1170,7 +1170,7 @@ func TestStartAndShutdownWithErroringWork(t *testing.T) {
 
 	item := types.NamespacedName{Namespace: "", Name: "bar"}
 
-	impl := NewImplFull(&errorReconciler{}, ControllerOptions{
+	impl := NewContext(context.TODO(), &errorReconciler{}, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      &FakeStatsReporter{},
@@ -1229,7 +1229,7 @@ func (er *permanentErrorReconciler) Reconcile(context.Context, string) error {
 func TestStartAndShutdownWithPermanentErroringWork(t *testing.T) {
 	r := &permanentErrorReconciler{}
 	reporter := &FakeStatsReporter{}
-	impl := NewImplFull(r, ControllerOptions{
+	impl := NewContext(context.TODO(), r, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      reporter,
@@ -1297,7 +1297,7 @@ func TestStartAndShutdownWithRequeuingWork(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			r := &requeueAfterReconciler{duration: test.duration}
 			reporter := &FakeStatsReporter{}
-			impl := NewImplFull(r, ControllerOptions{
+			impl := NewContext(context.TODO(), r, ControllerOptions{
 				Logger:        TestLogger(t),
 				WorkQueueName: "Testing",
 				Reporter:      reporter,
@@ -1396,7 +1396,7 @@ func (*fakeStore) List() []interface{} {
 
 func TestImplGlobalResync(t *testing.T) {
 	r := &CountingReconciler{}
-	impl := NewImplFull(r, ControllerOptions{
+	impl := NewContext(context.TODO(), r, ControllerOptions{
 		Logger:        TestLogger(t),
 		WorkQueueName: "Testing",
 		Reporter:      &FakeStatsReporter{},