Repository: knative/pkg
Pull request #1906 (2020-11-14 11:59:22)

drainer support kprobe
Signed-off-by: runzexia <root@junwuhui.cn>
ref https://github.com/knative/eventing/issues/3911
https://github.com/knative/networking/blob/bf808e4cd3f37ea514d365ffb2bb5ffa908e141c/pkg/probe/handler.go#L47-L48

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1906#" title="Author self-approved">runzexia</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **tcnghia** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @tcnghia` in a comment when ready.

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

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

- **[network/OWNERS](https://github.com/knative/pkg/blob/master/network/OWNERS)**

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":["tcnghia"]} -->
runzexia: /cc @mattmoor 
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1906?src=pr&el=h1) Report
> Merging [#1906](https://codecov.io/gh/knative/pkg/pull/1906?src=pr&el=desc) (c2c342b) into [master](https://codecov.io/gh/knative/pkg/commit/615f17a0eb057b3c54a8183392490251cec74a11?el=desc) (615f17a) will **increase** coverage by `0.00%`.
> The diff coverage is `72.72%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/1906/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO)](https://codecov.io/gh/knative/pkg/pull/1906?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master    #1906   +/-   ##
=======================================
  Coverage   69.42%   69.42%           
=======================================
  Files         207      207           
  Lines        8739     8750   +11     
=======================================
+ Hits         6067     6075    +8     
- Misses       2402     2403    +1     
- Partials      270      272    +2     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1906?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [network/handlers/drain.go](https://codecov.io/gh/knative/pkg/pull/1906/diff?src=pr&el=tree#diff-bmV0d29yay9oYW5kbGVycy9kcmFpbi5nbw==) | `84.09% <70.00%> (-4.15%)` | :arrow_down: |
| [network/network.go](https://codecov.io/gh/knative/pkg/pull/1906/diff?src=pr&el=tree#diff-bmV0d29yay9uZXR3b3JrLmdv) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1906?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/1906?src=pr&el=footer). Last update [615f17a...c2c342b](https://codecov.io/gh/knative/pkg/pull/1906?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

mattmoor: cc @ZhiminXiang @tcnghia @nak3 
knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1906#pullrequestreview-530858829" title="Approved">nak3</a>*, *<a href="https://github.com/knative/pkg/pull/1906#" title="Author self-approved">runzexia</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **tcnghia** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @tcnghia` in a comment when ready.

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

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

- **[network/OWNERS](https://github.com/knative/pkg/blob/master/network/OWNERS)**

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

Patch:
diff --git a/network/handlers/drain.go b/network/handlers/drain.go
index 42a6f25a6..75919578c 100644
--- a/network/handlers/drain.go
+++ b/network/handlers/drain.go
@@ -47,7 +47,7 @@ var newTimer = func(d time.Duration) timer {
 }
 
 // Drainer wraps an inner http.Handler to support responding to kubelet
-// probes with a "200 OK" until the handler is told to Drain.
+// probes and KProbes with a "200 OK" until the handler is told to Drain.
 // When the Drainer is told to Drain, it will immediately start to fail
 // probes with a "500 shutting down", and the call will block until no
 // requests have been received for QuietPeriod (defaults to
@@ -83,6 +83,14 @@ func (d *Drainer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 		return
 	}
+	if network.IsKProbe(r) {
+		if d.draining() {
+			http.Error(w, "shutting down", http.StatusServiceUnavailable)
+		} else {
+			network.ServeKProbe(w, r)
+		}
+		return
+	}
 
 	d.reset()
 	d.Inner.ServeHTTP(w, r)
diff --git a/network/network.go b/network/network.go
index 66766266c..93e63399d 100644
--- a/network/network.go
+++ b/network/network.go
@@ -17,6 +17,7 @@ limitations under the License.
 package network
 
 import (
+	"fmt"
 	"net/http"
 	"strings"
 	"time"
@@ -54,6 +55,16 @@ const (
 	// included in request metrics.
 	ProbeHeaderName = "K-Network-Probe"
 
+	// ProbeHeaderValue is the value of a header that can be added to
+	// requests to probe the knative networking layer.  Requests
+	// with `K-Network-Probe` this value will not be passed to the user
+	// container or included in request metrics.
+	ProbeHeaderValue = "probe"
+
+	// HashHeaderName is the name of an internal header that Ingress controller
+	// uses to find out which version of the networking config is deployed.
+	HashHeaderName = "K-Network-Hash"
+
 	// Since K8s 1.8, prober requests have
 	//   User-Agent = "kube-probe/{major-version}.{minor-version}".
 	KubeProbeUAPrefix = "kube-probe/"
@@ -68,3 +79,19 @@ func IsKubeletProbe(r *http.Request) bool {
 	return strings.HasPrefix(r.Header.Get("User-Agent"), KubeProbeUAPrefix) ||
 		r.Header.Get(KubeletProbeHeaderName) != ""
 }
+
+// IsKProbe returns true if the request is a knatvie probe.
+func IsKProbe(r *http.Request) bool {
+	return r.Header.Get(ProbeHeaderName) == ProbeHeaderValue
+}
+
+// ServeKProbe serve KProbe requests.
+func ServeKProbe(w http.ResponseWriter, r *http.Request) {
+	hh := r.Header.Get(HashHeaderName)
+	if hh == "" {
+		http.Error(w, fmt.Sprintf("a probe request must contain a non-empty %q header", HashHeaderName), http.StatusBadRequest)
+		return
+	}
+	w.Header().Set(HashHeaderName, hh)
+	w.WriteHeader(http.StatusOK)
+}


Test patch:
diff --git a/network/handlers/drain_test.go b/network/handlers/drain_test.go
index fd200d7e4..dacf6eac7 100644
--- a/network/handlers/drain_test.go
+++ b/network/handlers/drain_test.go
@@ -236,6 +236,58 @@ func TestDrainMechanics(t *testing.T) {
 	}
 }
 
+func TestDrainerKProbe(t *testing.T) {
+	var (
+		w          http.ResponseWriter
+		req        = &http.Request{}
+		kprobehash = "hash"
+		kprobe     = &http.Request{
+			Header: http.Header{
+				network.ProbeHeaderName: []string{network.ProbeHeaderValue},
+				network.HashHeaderName:  []string{kprobehash},
+			},
+		}
+		kprobeerr = &http.Request{
+			Header: http.Header{
+				network.ProbeHeaderName: []string{network.ProbeHeaderValue},
+			},
+		}
+		cnt   = 0
+		inner = http.HandlerFunc(func(http.ResponseWriter, *http.Request) { cnt++ })
+	)
+	drainer := &Drainer{
+		Inner: inner,
+	}
+
+	// Works before Drain is called.
+	drainer.ServeHTTP(w, req)
+	drainer.ServeHTTP(w, req)
+	drainer.ServeHTTP(w, req)
+	if cnt != 3 {
+		t.Error("Inner handler was not properly invoked")
+	}
+
+	resp := httptest.NewRecorder()
+	drainer.ServeHTTP(resp, kprobe)
+	if got, want := resp.Code, http.StatusOK; got != want {
+		t.Errorf("Probe status = %d, wanted %d", got, want)
+	}
+
+	if got, want := resp.Header().Get(network.HashHeaderName), kprobehash; got != want {
+		t.Errorf("KProbe hash = %s, wanted %s", got, want)
+	}
+
+	resp = httptest.NewRecorder()
+	drainer.ServeHTTP(resp, kprobeerr)
+	if got, want := resp.Code, http.StatusBadRequest; got != want {
+		t.Errorf("Probe status = %d, wanted %d", got, want)
+	}
+
+	if cnt != 3 {
+		t.Error("Inner handler was not properly invoked")
+	}
+}
+
 func TestDefaultQuietPeriod(t *testing.T) {
 	nt := newTimer
 	t.Cleanup(func() {
diff --git a/network/network_test.go b/network/network_test.go
index dc0eb2eb2..ec3903e36 100644
--- a/network/network_test.go
+++ b/network/network_test.go
@@ -18,6 +18,7 @@ package network
 
 import (
 	"net/http"
+	"net/http/httptest"
 	"testing"
 )
 
@@ -42,3 +43,58 @@ func TestIsKubeletProbe(t *testing.T) {
 		t.Error("kubelet probe but not counted as such")
 	}
 }
+
+func TestIsKnativeProbe(t *testing.T) {
+	req, err := http.NewRequest(http.MethodGet, "http://example.com/", nil)
+	if err != nil {
+		t.Fatal("Error building request:", err)
+	}
+	if IsKProbe(req) {
+		t.Error("Not a knative probe but counted as such")
+	}
+	req.Header.Set(ProbeHeaderName, ProbeHeaderValue)
+	if !IsKProbe(req) {
+		t.Error("knative probe but not counted as such")
+	}
+	req.Header.Del(ProbeHeaderName)
+	if IsKProbe(req) {
+		t.Error("Not a knative probe but counted as such")
+	}
+	req.Header.Set(ProbeHeaderName, "no matter")
+	if IsKProbe(req) {
+		t.Error("Not a knative probe but counted as such")
+	}
+}
+
+func TestServeKProbe(t *testing.T) {
+	var (
+		kprobehash = "hash"
+		kprobe     = &http.Request{
+			Header: http.Header{
+				ProbeHeaderName: []string{ProbeHeaderValue},
+				HashHeaderName:  []string{kprobehash},
+			},
+		}
+		kprobeerr = &http.Request{
+			Header: http.Header{
+				ProbeHeaderName: []string{ProbeHeaderValue},
+			},
+		}
+	)
+
+	resp := httptest.NewRecorder()
+	ServeKProbe(resp, kprobe)
+	if got, want := resp.Code, http.StatusOK; got != want {
+		t.Errorf("Probe status = %d, wanted %d", got, want)
+	}
+
+	if got, want := resp.Header().Get(HashHeaderName), kprobehash; got != want {
+		t.Errorf("KProbe hash = %s, wanted %s", got, want)
+	}
+
+	resp = httptest.NewRecorder()
+	ServeKProbe(resp, kprobeerr)
+	if got, want := resp.Code, http.StatusBadRequest; got != want {
+		t.Errorf("Probe status = %d, wanted %d", got, want)
+	}
+}
diff --git a/resolver/addressable_resolver_test.go b/resolver/addressable_resolver_test.go
index 358dfcb58..ba8c3b1fd 100644
--- a/resolver/addressable_resolver_test.go
+++ b/resolver/addressable_resolver_test.go
@@ -95,7 +95,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			objects: []runtime.Object{
 				getAddressable(),
 			},
-			dest: duckv1beta1.Destination{Ref: getAddressableRef(),
+			dest: duckv1beta1.Destination{Ref: addressableRef(),
 				DeprecatedKind:       addressableKind,
 				DeprecatedName:       addressableName,
 				DeprecatedAPIVersion: addressableAPIVersion,
@@ -107,14 +107,14 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			objects: []runtime.Object{
 				getAddressable(),
 			},
-			dest:    duckv1beta1.Destination{Ref: getAddressableRef()},
+			dest:    duckv1beta1.Destination{Ref: addressableRef()},
 			wantURI: addressableDNS,
 		}, "ref with relative uri": {
 			objects: []runtime.Object{
 				getAddressable(),
 			},
 			dest: duckv1beta1.Destination{
-				Ref: getAddressableRef(),
+				Ref: addressableRef(),
 				URI: &apis.URL{
 					Path: "/foo",
 				},
@@ -125,7 +125,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 				getAddressable(),
 			},
 			dest: duckv1beta1.Destination{
-				Ref: getAddressableRef(),
+				Ref: addressableRef(),
 				URI: &apis.URL{
 					Path: "foo",
 				},
@@ -133,10 +133,10 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "ref ends with path and trailing slash and relative URI without leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndTrailingSlash(),
+				addressableWithPathAndTrailingSlash(),
 			},
 			dest: duckv1beta1.Destination{
-				Ref: getAddressableRef(),
+				Ref: addressableRef(),
 				URI: &apis.URL{
 					Path: "foo",
 				},
@@ -144,10 +144,10 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantURI: addressableDNSWithPathAndTrailingSlash + "foo",
 		}, "ref ends with path and trailing slash and relative URI with leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndTrailingSlash(),
+				addressableWithPathAndTrailingSlash(),
 			},
 			dest: duckv1beta1.Destination{
-				Ref: getAddressableRef(),
+				Ref: addressableRef(),
 				URI: &apis.URL{
 					Path: "/foo",
 				},
@@ -155,10 +155,10 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "ref ends with path and no trailing slash and relative URI without leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndNoTrailingSlash(),
+				addressableWithPathAndNoTrailingSlash(),
 			},
 			dest: duckv1beta1.Destination{
-				Ref: getAddressableRef(),
+				Ref: addressableRef(),
 				URI: &apis.URL{
 					Path: "foo",
 				},
@@ -166,10 +166,10 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "ref ends with path and no trailing slash and relative URI with leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndNoTrailingSlash(),
+				addressableWithPathAndNoTrailingSlash(),
 			},
 			dest: duckv1beta1.Destination{
-				Ref: getAddressableRef(),
+				Ref: addressableRef(),
 				URI: &apis.URL{
 					Path: "/foo",
 				},
@@ -180,7 +180,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 				getAddressable(),
 			},
 			dest: duckv1beta1.Destination{
-				Ref: getAddressableRef(),
+				Ref: addressableRef(),
 				URI: &apis.URL{
 					Scheme: "http",
 					Host:   "example.com",
@@ -230,7 +230,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "[apiVersion, kind, name] ends with path and trailing slash and relative URI without leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndTrailingSlash(),
+				addressableWithPathAndTrailingSlash(),
 			},
 			dest: duckv1beta1.Destination{
 				DeprecatedKind:       addressableKind,
@@ -244,7 +244,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantURI: addressableDNSWithPathAndTrailingSlash + "foo",
 		}, "[apiVersion, kind, name] ends with path and trailing slash and relative URI with leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndTrailingSlash(),
+				addressableWithPathAndTrailingSlash(),
 			},
 			dest: duckv1beta1.Destination{
 				DeprecatedKind:       addressableKind,
@@ -258,7 +258,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "[apiVersion, kind, name] ends with path and no trailing slash and relative URI without leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndNoTrailingSlash(),
+				addressableWithPathAndNoTrailingSlash(),
 			},
 			dest: duckv1beta1.Destination{
 				DeprecatedKind:       addressableKind,
@@ -272,7 +272,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "[apiVersion, kind, name] ends with path and no trailing slash and relative URI with leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndNoTrailingSlash(),
+				addressableWithPathAndNoTrailingSlash(),
 			},
 			dest: duckv1beta1.Destination{
 				DeprecatedKind:       addressableKind,
@@ -303,31 +303,31 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 		},
 		"nil url": {
 			objects: []runtime.Object{
-				getAddressableNilURL(),
+				addressableNilURL(),
 			},
-			dest:    duckv1beta1.Destination{Ref: getUnaddressableRef()},
-			wantErr: fmt.Sprintf("URL missing in address of %+v", getUnaddressableRef()),
+			dest:    duckv1beta1.Destination{Ref: unaddressableRef()},
+			wantErr: fmt.Sprintf("URL missing in address of %+v", unaddressableRef()),
 		},
 		"nil address": {
 			objects: []runtime.Object{
-				getAddressableNilAddress(),
+				addressableNilAddress(),
 			},
-			dest:    duckv1beta1.Destination{Ref: getUnaddressableRef()},
-			wantErr: fmt.Sprintf("address not set for %+v", getUnaddressableRef()),
+			dest:    duckv1beta1.Destination{Ref: unaddressableRef()},
+			wantErr: fmt.Sprintf("address not set for %+v", unaddressableRef()),
 		}, "missing host": {
 			objects: []runtime.Object{
-				getAddressableNoHostURL(),
+				addressableNoHostURL(),
 			},
-			dest:    duckv1beta1.Destination{Ref: getUnaddressableRef()},
-			wantErr: fmt.Sprintf("hostname missing in address of %+v", getUnaddressableRef()),
+			dest:    duckv1beta1.Destination{Ref: unaddressableRef()},
+			wantErr: fmt.Sprintf("hostname missing in address of %+v", unaddressableRef()),
 		}, "missing status": {
 			objects: []runtime.Object{
-				getAddressableNoStatus(),
+				addressableNoStatus(),
 			},
-			dest:    duckv1beta1.Destination{Ref: getUnaddressableRef()},
-			wantErr: fmt.Sprintf("address not set for %+v", getUnaddressableRef()),
+			dest:    duckv1beta1.Destination{Ref: unaddressableRef()},
+			wantErr: fmt.Sprintf("address not set for %+v", unaddressableRef()),
 		}, "notFound": {
-			dest:    duckv1beta1.Destination{Ref: getUnaddressableRef()},
+			dest:    duckv1beta1.Destination{Ref: unaddressableRef()},
 			wantErr: fmt.Sprintf("%s %q not found", unaddressableResource, unaddressableName),
 		}}
 
@@ -395,20 +395,20 @@ func TestGetURIDestinationV1(t *testing.T) {
 			objects: []runtime.Object{
 				getAddressable(),
 			},
-			dest:    duckv1.Destination{Ref: getAddressableKnativeRef()},
+			dest:    duckv1.Destination{Ref: addressableKnativeRef()},
 			wantURI: addressableDNS,
 		}, "happy ref to k8s service": {
 			objects: []runtime.Object{
 				getAddressable(),
 			},
-			dest:    duckv1.Destination{Ref: getK8SServiceRef()},
+			dest:    duckv1.Destination{Ref: k8sServiceRef()},
 			wantURI: "http://testsink.testnamespace.svc.cluster.local/",
 		}, "ref with relative uri": {
 			objects: []runtime.Object{
 				getAddressable(),
 			},
 			dest: duckv1.Destination{
-				Ref: getAddressableKnativeRef(),
+				Ref: addressableKnativeRef(),
 				URI: &apis.URL{
 					Path: "/foo",
 				},
@@ -419,7 +419,7 @@ func TestGetURIDestinationV1(t *testing.T) {
 				getAddressable(),
 			},
 			dest: duckv1.Destination{
-				Ref: getAddressableKnativeRef(),
+				Ref: addressableKnativeRef(),
 				URI: &apis.URL{
 					Path: "foo",
 				},
@@ -427,10 +427,10 @@ func TestGetURIDestinationV1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "ref ends with path and trailing slash and relative URI without leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndTrailingSlash(),
+				addressableWithPathAndTrailingSlash(),
 			},
 			dest: duckv1.Destination{
-				Ref: getAddressableKnativeRef(),
+				Ref: addressableKnativeRef(),
 				URI: &apis.URL{
 					Path: "foo",
 				},
@@ -438,10 +438,10 @@ func TestGetURIDestinationV1(t *testing.T) {
 			wantURI: addressableDNSWithPathAndTrailingSlash + "foo",
 		}, "ref ends with path and trailing slash and relative URI with leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndTrailingSlash(),
+				addressableWithPathAndTrailingSlash(),
 			},
 			dest: duckv1.Destination{
-				Ref: getAddressableKnativeRef(),
+				Ref: addressableKnativeRef(),
 				URI: &apis.URL{
 					Path: "/foo",
 				},
@@ -449,10 +449,10 @@ func TestGetURIDestinationV1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "ref ends with path and no trailing slash and relative URI without leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndNoTrailingSlash(),
+				addressableWithPathAndNoTrailingSlash(),
 			},
 			dest: duckv1.Destination{
-				Ref: getAddressableKnativeRef(),
+				Ref: addressableKnativeRef(),
 				URI: &apis.URL{
 					Path: "foo",
 				},
@@ -460,10 +460,10 @@ func TestGetURIDestinationV1(t *testing.T) {
 			wantURI: addressableDNS + "/foo",
 		}, "ref ends with path and no trailing slash and relative URI with leading slash ": {
 			objects: []runtime.Object{
-				getAddressableWithPathAndNoTrailingSlash(),
+				addressableWithPathAndNoTrailingSlash(),
 			},
 			dest: duckv1.Destination{
-				Ref: getAddressableKnativeRef(),
+				Ref: addressableKnativeRef(),
 				URI: &apis.URL{
 					Path: "/foo",
 				},
@@ -474,7 +474,7 @@ func TestGetURIDestinationV1(t *testing.T) {
 				getAddressable(),
 			},
 			dest: duckv1.Destination{
-				Ref: getAddressableKnativeRef(),
+				Ref: addressableKnativeRef(),
 				URI: &apis.URL{
 					Scheme: "http",
 					Host:   "example.com",
@@ -485,31 +485,31 @@ func TestGetURIDestinationV1(t *testing.T) {
 		},
 		"nil url": {
 			objects: []runtime.Object{
-				getAddressableNilURL(),
+				addressableNilURL(),
 			},
-			dest:    duckv1.Destination{Ref: getUnaddressableKnativeRef()},
-			wantErr: fmt.Sprintf("URL missing in address of %+v", getUnaddressableRef()),
+			dest:    duckv1.Destination{Ref: unaddressableKnativeRef()},
+			wantErr: fmt.Sprintf("URL missing in address of %+v", unaddressableRef()),
 		},
 		"nil address": {
 			objects: []runtime.Object{
-				getAddressableNilAddress(),
+				addressableNilAddress(),
 			},
-			dest:    duckv1.Destination{Ref: getUnaddressableKnativeRef()},
-			wantErr: fmt.Sprintf("address not set for %+v", getUnaddressableRef()),
+			dest:    duckv1.Destination{Ref: unaddressableKnativeRef()},
+			wantErr: fmt.Sprintf("address not set for %+v", unaddressableRef()),
 		}, "missing host": {
 			objects: []runtime.Object{
-				getAddressableNoHostURL(),
+				addressableNoHostURL(),
 			},
-			dest:    duckv1.Destination{Ref: getUnaddressableKnativeRef()},
-			wantErr: fmt.Sprintf("hostname missing in address of %+v", getUnaddressableRef()),
+			dest:    duckv1.Destination{Ref: unaddressableKnativeRef()},
+			wantErr: fmt.Sprintf("hostname missing in address of %+v", unaddressableRef()),
 		}, "missing status": {
 			objects: []runtime.Object{
-				getAddressableNoStatus(),
+				addressableNoStatus(),
 			},
-			dest:    duckv1.Destination{Ref: getUnaddressableKnativeRef()},
-			wantErr: fmt.Sprintf("address not set for %+v", getUnaddressableRef()),
+			dest:    duckv1.Destination{Ref: unaddressableKnativeRef()},
+			wantErr: fmt.Sprintf("address not set for %+v", unaddressableRef()),
 		}, "notFound": {
-			dest:    duckv1.Destination{Ref: getUnaddressableKnativeRef()},
+			dest:    duckv1.Destination{Ref: unaddressableKnativeRef()},
 			wantErr: fmt.Sprintf("%s %q not found", unaddressableResource, unaddressableName),
 		}}
 
@@ -548,10 +548,10 @@ func TestURIFromObjectReferenceErrors(t *testing.T) {
 	}{"nil": {
 		wantErr: "ref is nil",
 	}, "fail tracker with bad object": {
-		ref:     getInvalidObjectReference(),
+		ref:     invalidObjectRef(),
 		wantErr: `sinks.duck.knative.dev "testsink" not found`,
 	}, "fail get": {
-		ref:     getAddressableRef(),
+		ref:     addressableRef(),
 		wantErr: `sinks.duck.knative.dev "testsink" not found`,
 	}}
 
@@ -563,21 +563,17 @@ func TestURIFromObjectReferenceErrors(t *testing.T) {
 
 			// Run it twice since this should be idempotent. URI Resolver should
 			// not modify the cache's copy.
-			_, _ = r.URIFromObjectReference(ctx, tc.ref, getAddressable())
-			_, gotErr := r.URIFromObjectReference(ctx, tc.ref, getAddressable())
+			_, err1 := r.URIFromObjectReference(ctx, tc.ref, getAddressable())
+			_, err2 := r.URIFromObjectReference(ctx, tc.ref, getAddressable())
 
-			if gotErr != nil {
-				if tc.wantErr != "" {
-					if got, want := gotErr.Error(), tc.wantErr; got != want {
-						t.Errorf("Unexpected error (-want, +got) =\n%s", cmp.Diff(want, got))
-					}
-				} else {
-					t.Error("Unexpected error:", gotErr)
-				}
-				return
+			if err2 == nil {
+				t.Fatal("Expected failure")
+			}
+			if got, want := err2.Error(), tc.wantErr; got != want {
+				t.Errorf("Unexpected error (-want, +got) =\n%s", cmp.Diff(want, got))
 			}
-			if tc.wantErr != "" {
-				t.Errorf("Expected error: %q but got none", tc.wantErr)
+			if !cmp.Equal(err1, err2) {
+				t.Errorf("Idempotency fail: first err = %v, second err = %v", err1, err2)
 			}
 		})
 	}
@@ -601,7 +597,7 @@ func getAddressable() *unstructured.Unstructured {
 	}
 }
 
-func getAddressableWithPathAndTrailingSlash() *unstructured.Unstructured {
+func addressableWithPathAndTrailingSlash() *unstructured.Unstructured {
 	return &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"apiVersion": addressableAPIVersion,
@@ -619,7 +615,7 @@ func getAddressableWithPathAndTrailingSlash() *unstructured.Unstructured {
 	}
 }
 
-func getAddressableWithPathAndNoTrailingSlash() *unstructured.Unstructured {
+func addressableWithPathAndNoTrailingSlash() *unstructured.Unstructured {
 	return &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"apiVersion": addressableAPIVersion,
@@ -637,7 +633,7 @@ func getAddressableWithPathAndNoTrailingSlash() *unstructured.Unstructured {
 	}
 }
 
-func getAddressableNoStatus() *unstructured.Unstructured {
+func addressableNoStatus() *unstructured.Unstructured {
 	return &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"apiVersion": unaddressableAPIVersion,
@@ -650,7 +646,7 @@ func getAddressableNoStatus() *unstructured.Unstructured {
 	}
 }
 
-func getAddressableNilAddress() *unstructured.Unstructured {
+func addressableNilAddress() *unstructured.Unstructured {
 	return &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"apiVersion": unaddressableAPIVersion,
@@ -666,7 +662,7 @@ func getAddressableNilAddress() *unstructured.Unstructured {
 	}
 }
 
-func getAddressableNilURL() *unstructured.Unstructured {
+func addressableNilURL() *unstructured.Unstructured {
 	return &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"apiVersion": unaddressableAPIVersion,
@@ -684,7 +680,7 @@ func getAddressableNilURL() *unstructured.Unstructured {
 	}
 }
 
-func getAddressableNoHostURL() *unstructured.Unstructured {
+func addressableNoHostURL() *unstructured.Unstructured {
 	return &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"apiVersion": unaddressableAPIVersion,
@@ -702,7 +698,7 @@ func getAddressableNoHostURL() *unstructured.Unstructured {
 	}
 }
 
-func getInvalidObjectReference() *corev1.ObjectReference {
+func invalidObjectRef() *corev1.ObjectReference {
 	return &corev1.ObjectReference{
 		Kind:       addressableKind,
 		Name:       addressableName,
@@ -712,7 +708,7 @@ func getInvalidObjectReference() *corev1.ObjectReference {
 
 }
 
-func getK8SServiceRef() *duckv1.KReference {
+func k8sServiceRef() *duckv1.KReference {
 	return &duckv1.KReference{
 		Kind:       "Service",
 		Name:       addressableName,
@@ -722,7 +718,7 @@ func getK8SServiceRef() *duckv1.KReference {
 
 }
 
-func getAddressableKnativeRef() *duckv1.KReference {
+func addressableKnativeRef() *duckv1.KReference {
 	return &duckv1.KReference{
 		Kind:       addressableKind,
 		Name:       addressableName,
@@ -731,7 +727,7 @@ func getAddressableKnativeRef() *duckv1.KReference {
 	}
 }
 
-func getAddressableRef() *corev1.ObjectReference {
+func addressableRef() *corev1.ObjectReference {
 	return &corev1.ObjectReference{
 		Kind:       addressableKind,
 		Name:       addressableName,
@@ -740,7 +736,7 @@ func getAddressableRef() *corev1.ObjectReference {
 	}
 }
 
-func getUnaddressableKnativeRef() *duckv1.KReference {
+func unaddressableKnativeRef() *duckv1.KReference {
 	return &duckv1.KReference{
 		Kind:       unaddressableKind,
 		Name:       unaddressableName,
@@ -749,7 +745,7 @@ func getUnaddressableKnativeRef() *duckv1.KReference {
 	}
 }
 
-func getUnaddressableRef() *corev1.ObjectReference {
+func unaddressableRef() *corev1.ObjectReference {
 	return &corev1.ObjectReference{
 		Kind:       unaddressableKind,
 		Name:       unaddressableName,