Repository: knative/serving
Pull request #8309 (2020-06-12 16:55:51)

Remove the pointer return from makeIngressSpec
There's no reason to return a pointer since it's deref'd immideately.
Also gives a chance not to escape (though it will anyway outside tests).
Other minor nits.

/assign @tcnghia @ZhiminXiang 

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

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

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

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:

- ~~[pkg/reconciler/route/OWNERS](https://github.com/knative/serving/blob/master/pkg/reconciler/route/OWNERS)~~ [vagababov]

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/cmd/queue/execprobe.go b/cmd/queue/execprobe.go
new file mode 100644
index 000000000..cefb70576
--- /dev/null
+++ b/cmd/queue/execprobe.go
@@ -0,0 +1,134 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+	"os"
+	"strconv"
+	"time"
+
+	"k8s.io/apimachinery/pkg/util/wait"
+	"knative.dev/serving/pkg/network"
+	"knative.dev/serving/pkg/queue"
+	"knative.dev/serving/pkg/queue/health"
+	"knative.dev/serving/pkg/queue/readiness"
+)
+
+const (
+	healthURLPrefix = "http://127.0.0.1:"
+
+	// The 25 millisecond retry interval is an unscientific compromise between wanting to get
+	// started as early as possible while still wanting to give the container some breathing
+	// room to get up and running.
+	aggressivePollInterval = 25 * time.Millisecond
+)
+
+// As well as running as a long-running proxy server, the Queue Proxy can be
+// run as an exec probe if the `--probe-period` flag is passed.
+//
+// In this mode, the exec probe (repeatedly) sends an HTTP request to the Queue
+// Proxy server with a Probe header. The handler for this probe request
+// (knativeProbeHandler) then performs the actual health check against the user
+// container. The exec probe binary exits 0 (success) if it eventually gets a
+// 200 status code back from the knativeProbeHandler, or 1 (fail) if it never
+// does.
+//
+// The reason we use an exec probe to hit an HTTP endpoint on the Queue
+// Proxy to then run the actual probe against the user container rather than
+// using an HTTP probe directly is because an exec probe can be launched
+// immediately after the container starts, whereas an HTTP probe would
+// initially race with the server starting up and fail. The minimum retry
+// period after this failure in upstream kubernetes is a second. The exec
+// probe, on the other hand, automatically polls on an aggressivePollInterval
+// until the HTTP endpoint responds with success. This allows us to get an
+// initial readiness result much faster than the effective upstream Kubernetes
+// minimum of 1 second.
+func standaloneProbeMain(timeout int) (exitCode int) {
+	queueServingPort, err := strconv.Atoi(os.Getenv("QUEUE_SERVING_PORT"))
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "parse queue port:", err)
+		return 1
+	}
+
+	if err := probeQueueHealthPath(timeout, queueServingPort); err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		return 1
+	}
+
+	return 0
+}
+
+func probeQueueHealthPath(timeoutSeconds, queueServingPort int) error {
+	if queueServingPort <= 0 {
+		return fmt.Errorf("port must be a positive value, got %d", queueServingPort)
+	}
+
+	url := healthURLPrefix + strconv.Itoa(queueServingPort)
+	timeoutDuration := readiness.PollTimeout
+	if timeoutSeconds != 0 {
+		timeoutDuration = time.Duration(timeoutSeconds) * time.Second
+	}
+
+	req, err := http.NewRequest(http.MethodGet, url, nil)
+	if err != nil {
+		return fmt.Errorf("probe failed: error creating request: %w", err)
+	}
+
+	// Add the header to indicate this is a probe request.
+	req.Header.Add(network.ProbeHeaderName, queue.Name)
+	req.Header.Add(network.UserAgentKey, network.QueueProxyUserAgent)
+
+	httpClient := &http.Client{
+		Timeout: timeoutDuration,
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), timeoutDuration)
+	defer cancel()
+
+	var lastErr error
+	// Using PollImmediateUntil instead of PollImmediate because if timeout is reached while waiting for first
+	// invocation of conditionFunc, it exits immediately without trying for a second time.
+	timeoutErr := wait.PollImmediateUntil(aggressivePollInterval, func() (bool, error) {
+		res, lastErr := httpClient.Do(req)
+		if lastErr != nil {
+			// Return nil error for retrying
+			return false, nil
+		}
+		defer res.Body.Close()
+
+		// fail readiness immediately rather than retrying if we get a header indicating we're shutting down.
+		if health.IsHTTPProbeShuttingDown(res) {
+			return false, errors.New("failing probe deliberately for shutdown")
+		}
+
+		return health.IsHTTPProbeReady(res), nil
+	}, ctx.Done())
+
+	if lastErr != nil {
+		return fmt.Errorf("failed to probe: %w", lastErr)
+	}
+
+	// An http.StatusOK was never returned during probing
+	if timeoutErr != nil {
+		return errors.New("probe returned not ready")
+	}
+
+	return nil
+}
diff --git a/cmd/queue/main.go b/cmd/queue/main.go
index b0334470e..176d26550 100644
--- a/cmd/queue/main.go
+++ b/cmd/queue/main.go
@@ -18,7 +18,6 @@ package main
 
 import (
 	"context"
-	"errors"
 	"flag"
 	"fmt"
 	"net"
@@ -36,7 +35,6 @@ import (
 	"go.uber.org/zap"
 
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/wait"
 
 	"knative.dev/networking/pkg/apis/networking"
 	pkglogging "knative.dev/pkg/logging"
@@ -59,14 +57,8 @@ import (
 )
 
 const (
-	badProbeTemplate   = "unexpected probe header value: %s"
-	failingHealthcheck = "failing healthcheck"
-
-	healthURLPrefix = "http://127.0.0.1:"
-	// The 25 millisecond retry interval is an unscientific compromise between wanting to get
-	// started as early as possible while still wanting to give the container some breathing
-	// room to get up and running.
-	aggressivePollInterval = 25 * time.Millisecond
+	badProbeTemplate = "unexpected probe header value: %s"
+
 	// reportingPeriod is the interval of time between reporting stats by queue proxy.
 	reportingPeriod = 1 * time.Second
 )
@@ -197,81 +189,12 @@ func knativeProbeHandler(healthState *health.State, prober func() bool, isAggres
 	}
 }
 
-func probeQueueHealthPath(timeoutSeconds, queueServingPort int) error {
-	if queueServingPort <= 0 {
-		return fmt.Errorf("port must be a positive value, got %d", queueServingPort)
-	}
-
-	url := healthURLPrefix + strconv.Itoa(queueServingPort)
-	timeoutDuration := readiness.PollTimeout
-	if timeoutSeconds != 0 {
-		timeoutDuration = time.Duration(timeoutSeconds) * time.Second
-	}
-
-	req, err := http.NewRequest(http.MethodGet, url, nil)
-	if err != nil {
-		return fmt.Errorf("probe failed: error creating request: %w", err)
-	}
-	// Add the header to indicate this is a probe request.
-	req.Header.Add(network.ProbeHeaderName, queue.Name)
-	req.Header.Add(network.UserAgentKey, network.QueueProxyUserAgent)
-
-	httpClient := &http.Client{
-		Timeout: timeoutDuration,
-	}
-	ctx, cancel := context.WithTimeout(context.Background(), timeoutDuration)
-	defer cancel()
-
-	var lastErr error
-	// Using PollImmediateUntil instead of PollImmediate because if timeout is reached while waiting for first
-	// invocation of conditionFunc, it exits immediately without trying for a second time.
-	timeoutErr := wait.PollImmediateUntil(aggressivePollInterval, func() (bool, error) {
-		res, lastErr := httpClient.Do(req)
-		if lastErr != nil {
-			// Return nil error for retrying
-			return false, nil
-		}
-		defer res.Body.Close()
-
-		// fail readiness immediately rather than retrying if we get a header indicating we're shutting down.
-		if health.IsHTTPProbeShuttingDown(res) {
-			return false, errors.New("failing probe deliberately for shutdown")
-		}
-
-		return health.IsHTTPProbeReady(res), nil
-	}, ctx.Done())
-
-	if lastErr != nil {
-		return fmt.Errorf("failed to probe: %w", lastErr)
-	}
-
-	// An http.StatusOK was never returned during probing
-	if timeoutErr != nil {
-		return errors.New("probe returned not ready")
-	}
-
-	return nil
-}
-
 func main() {
 	flag.Parse()
 
 	// If this is set, we run as a standalone binary to probe the queue-proxy.
 	if *readinessProbeTimeout >= 0 {
-		queueServingPort, err := strconv.Atoi(os.Getenv("QUEUE_SERVING_PORT"))
-		if err != nil {
-			// used instead of the logger to produce a concise event message
-			fmt.Fprintln(os.Stderr, "parse queue port:", err)
-			os.Exit(1)
-		}
-
-		if err := probeQueueHealthPath(*readinessProbeTimeout, queueServingPort); err != nil {
-			// used instead of the logger to produce a concise event message
-			fmt.Fprintln(os.Stderr, err)
-			os.Exit(1)
-		}
-
-		os.Exit(0)
+		os.Exit(standaloneProbeMain(*readinessProbeTimeout))
 	}
 
 	// Parse the environment.
diff --git a/pkg/network/stats.go b/pkg/network/stats.go
index d4be2f734..bff893e1d 100644
--- a/pkg/network/stats.go
+++ b/pkg/network/stats.go
@@ -130,8 +130,8 @@ func (s *RequestStats) HandleEvent(event ReqEvent) {
 	}
 }
 
-// Report returns averageConcurrency, averageProxiedConcurrency, requestCount and proxiedCount
-// relative to the given time. The state will be reset for another reporting cycle afterwards.
+// Report returns a RequestStatsReport relative to the given time. The state
+// will be reset for another reporting cycle afterwards.
 func (s *RequestStats) Report(now time.Time) RequestStatsReport {
 	s.mux.Lock()
 	defer s.mux.Unlock()
diff --git a/pkg/reconciler/route/resources/ingress.go b/pkg/reconciler/route/resources/ingress.go
index 2885b3d9b..b4b50abdd 100644
--- a/pkg/reconciler/route/resources/ingress.go
+++ b/pkg/reconciler/route/resources/ingress.go
@@ -114,7 +114,7 @@ func MakeIngressSpec(
 			if err != nil {
 				return netv1alpha1.IngressSpec{}, err
 			}
-			rule := *makeIngressRule([]string{domain}, r.Namespace, visibility, targets[name])
+			rule := makeIngressRule([]string{domain}, r.Namespace, visibility, targets[name])
 			if networkConfig.TagHeaderBasedRouting {
 				if rule.HTTP.Paths[0].AppendHeaders == nil {
 					rule.HTTP.Paths[0].AppendHeaders = make(map[string]string)
@@ -203,8 +203,9 @@ func makeACMEIngressPaths(challenges map[string]netv1alpha1.HTTP01Challenge, dom
 	return paths
 }
 
-func makeIngressRule(domains []string, ns string, visibility netv1alpha1.IngressVisibility, targets traffic.RevisionTargets) *netv1alpha1.IngressRule {
-	return &netv1alpha1.IngressRule{
+func makeIngressRule(domains []string, ns string,
+	visibility netv1alpha1.IngressVisibility, targets traffic.RevisionTargets) netv1alpha1.IngressRule {
+	return netv1alpha1.IngressRule{
 		Hosts:      domains,
 		Visibility: visibility,
 		HTTP: &netv1alpha1.HTTPIngressRuleValue{
diff --git a/pkg/reconciler/route/resources/service.go b/pkg/reconciler/route/resources/service.go
index 1c50e73b4..e0163bea8 100644
--- a/pkg/reconciler/route/resources/service.go
+++ b/pkg/reconciler/route/resources/service.go
@@ -188,7 +188,7 @@ func GetDesiredServiceNames(ctx context.Context, route *v1.Route) (sets.String,
 	for _, t := range traffic {
 		serviceName, err := domains.HostnameFromTemplate(ctx, route.Name, t.Tag)
 		if err != nil {
-			return sets.String{}, err
+			return nil, err
 		}
 		names.Insert(serviceName)
 	}


Test patch:
diff --git a/cmd/queue/execprobe_test.go b/cmd/queue/execprobe_test.go
new file mode 100644
index 000000000..50acbb8aa
--- /dev/null
+++ b/cmd/queue/execprobe_test.go
@@ -0,0 +1,199 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp"
+	"knative.dev/pkg/ptr"
+	"knative.dev/serving/pkg/network"
+)
+
+func TestProbeQueueInvalidPort(t *testing.T) {
+	if err := probeQueueHealthPath(1, 0); err == nil {
+		t.Error("Expected error, got nil")
+	} else if diff := cmp.Diff(err.Error(), "port must be a positive value, got 0"); diff != "" {
+		t.Errorf("Unexpected not ready message: %s", diff)
+	}
+}
+
+func TestProbeQueueConnectionFailure(t *testing.T) {
+	if err := probeQueueHealthPath(1, 12345); err == nil {
+		t.Error("Expected error, got nil")
+	}
+}
+
+func TestProbeQueueNotReady(t *testing.T) {
+	queueProbed := ptr.Int32(0)
+	ts := newProbeTestServer(func(w http.ResponseWriter) {
+		atomic.AddInt32(queueProbed, 1)
+		w.WriteHeader(http.StatusBadRequest)
+	})
+
+	defer ts.Close()
+
+	u, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
+	}
+
+	port, err := strconv.Atoi(u.Port())
+	if err != nil {
+		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
+	}
+
+	err = probeQueueHealthPath(1, port)
+
+	if diff := cmp.Diff(err.Error(), "probe returned not ready"); diff != "" {
+		t.Errorf("Unexpected not ready message: %s", diff)
+	}
+
+	if atomic.LoadInt32(queueProbed) == 0 {
+		t.Error("Expected the queue proxy server to be probed")
+	}
+}
+
+func TestProbeQueueShuttingDownFailsFast(t *testing.T) {
+	ts := newProbeTestServer(func(w http.ResponseWriter) {
+		w.WriteHeader(http.StatusGone)
+	})
+
+	defer ts.Close()
+
+	u, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
+	}
+
+	port, err := strconv.Atoi(u.Port())
+	if err != nil {
+		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
+	}
+
+	start := time.Now()
+	if err = probeQueueHealthPath(1, port); err == nil {
+		t.Error("probeQueueHealthPath did not fail")
+	}
+
+	// if fails due to timeout and not cancelation, then it took too long
+	if time.Since(start) >= 1*time.Second {
+		t.Error("took too long to fail")
+	}
+}
+
+func TestProbeQueueReady(t *testing.T) {
+	queueProbed := ptr.Int32(0)
+	ts := newProbeTestServer(func(w http.ResponseWriter) {
+		atomic.AddInt32(queueProbed, 1)
+		w.WriteHeader(http.StatusOK)
+	})
+
+	defer ts.Close()
+
+	u, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
+	}
+
+	port, err := strconv.Atoi(u.Port())
+	if err != nil {
+		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
+	}
+
+	if err = probeQueueHealthPath(1, port); err != nil {
+		t.Errorf("probeQueueHealthPath(%d, 1s) = %s", port, err)
+	}
+
+	if atomic.LoadInt32(queueProbed) == 0 {
+		t.Error("Expected the queue proxy server to be probed")
+	}
+}
+
+func TestProbeQueueTimeout(t *testing.T) {
+	queueProbed := ptr.Int32(0)
+	ts := newProbeTestServer(func(w http.ResponseWriter) {
+		atomic.AddInt32(queueProbed, 1)
+		time.Sleep(2 * time.Second)
+		w.WriteHeader(http.StatusOK)
+	})
+
+	defer ts.Close()
+
+	u, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
+	}
+
+	port, err := strconv.Atoi(u.Port())
+	if err != nil {
+		t.Fatalf("failed to convert port(%s) to int", u.Port())
+	}
+
+	timeout := 1
+	if err = probeQueueHealthPath(timeout, port); err == nil {
+		t.Errorf("Expected probeQueueHealthPath(%d, %v) to return timeout error", port, timeout)
+	}
+
+	ts.Close()
+
+	if atomic.LoadInt32(queueProbed) == 0 {
+		t.Error("Expected the queue proxy server to be probed")
+	}
+}
+
+func TestProbeQueueDelayedReady(t *testing.T) {
+	count := ptr.Int32(0)
+	ts := newProbeTestServer(func(w http.ResponseWriter) {
+		if atomic.AddInt32(count, 1) < 9 {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+	})
+
+	defer ts.Close()
+
+	u, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
+	}
+
+	port, err := strconv.Atoi(u.Port())
+	if err != nil {
+		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
+	}
+
+	timeout := 0
+	if err := probeQueueHealthPath(timeout, port); err != nil {
+		t.Errorf("probeQueueHealthPath(%d) = %s", port, err)
+	}
+}
+
+func newProbeTestServer(f func(w http.ResponseWriter)) *httptest.Server {
+	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get(network.UserAgentKey) == network.QueueProxyUserAgent {
+			f(w)
+		}
+	}))
+}
diff --git a/cmd/queue/main_test.go b/cmd/queue/main_test.go
index 5d7f5b6db..a52204edc 100644
--- a/cmd/queue/main_test.go
+++ b/cmd/queue/main_test.go
@@ -22,16 +22,13 @@ import (
 	"net/http/httptest"
 	"net/http/httputil"
 	"net/url"
-	"strconv"
 	"strings"
-	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"go.opencensus.io/plugin/ochttp"
 	pkgnet "knative.dev/pkg/network"
-	"knative.dev/pkg/ptr"
 	"knative.dev/pkg/tracing"
 	tracingconfig "knative.dev/pkg/tracing/config"
 	tracetesting "knative.dev/pkg/tracing/testing"
@@ -149,166 +146,6 @@ func TestProbeHandler(t *testing.T) {
 	}
 }
 
-func TestProbeQueueInvalidPort(t *testing.T) {
-	if err := probeQueueHealthPath(1, 0); err == nil {
-		t.Error("Expected error, got nil")
-	} else if diff := cmp.Diff(err.Error(), "port must be a positive value, got 0"); diff != "" {
-		t.Errorf("Unexpected not ready message: %s", diff)
-	}
-}
-
-func TestProbeQueueConnectionFailure(t *testing.T) {
-	if err := probeQueueHealthPath(1, 12345); err == nil {
-		t.Error("Expected error, got nil")
-	}
-}
-
-func TestProbeQueueNotReady(t *testing.T) {
-	queueProbed := ptr.Int32(0)
-	ts := newProbeTestServer(func(w http.ResponseWriter) {
-		atomic.AddInt32(queueProbed, 1)
-		w.WriteHeader(http.StatusBadRequest)
-	})
-
-	defer ts.Close()
-
-	u, err := url.Parse(ts.URL)
-	if err != nil {
-		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
-	}
-
-	port, err := strconv.Atoi(u.Port())
-	if err != nil {
-		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
-	}
-
-	err = probeQueueHealthPath(1, port)
-
-	if diff := cmp.Diff(err.Error(), "probe returned not ready"); diff != "" {
-		t.Errorf("Unexpected not ready message: %s", diff)
-	}
-
-	if atomic.LoadInt32(queueProbed) == 0 {
-		t.Error("Expected the queue proxy server to be probed")
-	}
-}
-
-func TestProbeQueueShuttingDownFailsFast(t *testing.T) {
-	ts := newProbeTestServer(func(w http.ResponseWriter) {
-		w.WriteHeader(http.StatusGone)
-	})
-
-	defer ts.Close()
-
-	u, err := url.Parse(ts.URL)
-	if err != nil {
-		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
-	}
-
-	port, err := strconv.Atoi(u.Port())
-	if err != nil {
-		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
-	}
-
-	start := time.Now()
-	if err = probeQueueHealthPath(1, port); err == nil {
-		t.Error("probeQueueHealthPath did not fail")
-	}
-
-	// if fails due to timeout and not cancelation, then it took too long
-	if time.Since(start) >= 1*time.Second {
-		t.Error("took too long to fail")
-	}
-}
-
-func TestProbeQueueReady(t *testing.T) {
-	queueProbed := ptr.Int32(0)
-	ts := newProbeTestServer(func(w http.ResponseWriter) {
-		atomic.AddInt32(queueProbed, 1)
-		w.WriteHeader(http.StatusOK)
-	})
-
-	defer ts.Close()
-
-	u, err := url.Parse(ts.URL)
-	if err != nil {
-		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
-	}
-
-	port, err := strconv.Atoi(u.Port())
-	if err != nil {
-		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
-	}
-
-	if err = probeQueueHealthPath(1, port); err != nil {
-		t.Errorf("probeQueueHealthPath(%d, 1s) = %s", port, err)
-	}
-
-	if atomic.LoadInt32(queueProbed) == 0 {
-		t.Error("Expected the queue proxy server to be probed")
-	}
-}
-
-func TestProbeQueueTimeout(t *testing.T) {
-	queueProbed := ptr.Int32(0)
-	ts := newProbeTestServer(func(w http.ResponseWriter) {
-		atomic.AddInt32(queueProbed, 1)
-		time.Sleep(2 * time.Second)
-		w.WriteHeader(http.StatusOK)
-	})
-
-	defer ts.Close()
-
-	u, err := url.Parse(ts.URL)
-	if err != nil {
-		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
-	}
-
-	port, err := strconv.Atoi(u.Port())
-	if err != nil {
-		t.Fatalf("failed to convert port(%s) to int", u.Port())
-	}
-
-	timeout := 1
-	if err = probeQueueHealthPath(timeout, port); err == nil {
-		t.Errorf("Expected probeQueueHealthPath(%d, %v) to return timeout error", port, timeout)
-	}
-
-	ts.Close()
-
-	if atomic.LoadInt32(queueProbed) == 0 {
-		t.Error("Expected the queue proxy server to be probed")
-	}
-}
-
-func TestProbeQueueDelayedReady(t *testing.T) {
-	count := ptr.Int32(0)
-	ts := newProbeTestServer(func(w http.ResponseWriter) {
-		if atomic.AddInt32(count, 1) < 9 {
-			w.WriteHeader(http.StatusBadRequest)
-			return
-		}
-		w.WriteHeader(http.StatusOK)
-	})
-
-	defer ts.Close()
-
-	u, err := url.Parse(ts.URL)
-	if err != nil {
-		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
-	}
-
-	port, err := strconv.Atoi(u.Port())
-	if err != nil {
-		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
-	}
-
-	timeout := 0
-	if err := probeQueueHealthPath(timeout, port); err != nil {
-		t.Errorf("probeQueueHealthPath(%d) = %s", port, err)
-	}
-}
-
 func TestQueueTraceSpans(t *testing.T) {
 	logger = TestLogger(t)
 	testcases := []struct {
@@ -509,11 +346,3 @@ func BenchmarkProxyHandler(b *testing.B) {
 		})
 	}
 }
-
-func newProbeTestServer(f func(w http.ResponseWriter)) *httptest.Server {
-	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.Header.Get(network.UserAgentKey) == network.QueueProxyUserAgent {
-			f(w)
-		}
-	}))
-}
diff --git a/pkg/reconciler/route/resources/ingress_test.go b/pkg/reconciler/route/resources/ingress_test.go
index 6e569b084..bb01652fc 100644
--- a/pkg/reconciler/route/resources/ingress_test.go
+++ b/pkg/reconciler/route/resources/ingress_test.go
@@ -50,7 +50,7 @@ const (
 	testIngressClass    = "test-ingress"
 )
 
-func TestMakeIngress_CorrectMetadata(t *testing.T) {
+func TestMakeIngressCorrectMetadata(t *testing.T) {
 	targets := map[string]traffic.RevisionTargets{}
 	ingressClass := "ng-ingress"
 	passdownIngressClass := "ok-ingress"
@@ -515,8 +515,8 @@ func TestMakeIngressRule_Vanilla(t *testing.T) {
 		Visibility: netv1alpha1.IngressVisibilityExternalIP,
 	}
 
-	if !cmp.Equal(&expected, rule) {
-		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(&expected, rule))
+	if !cmp.Equal(expected, rule) {
+		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(expected, rule))
 	}
 }
 
@@ -561,8 +561,8 @@ func TestMakeIngressRule_ZeroPercentTarget(t *testing.T) {
 		Visibility: netv1alpha1.IngressVisibilityExternalIP,
 	}
 
-	if !cmp.Equal(&expected, rule) {
-		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(&expected, rule))
+	if !cmp.Equal(expected, rule) {
+		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(expected, rule))
 	}
 }
 
@@ -607,8 +607,8 @@ func TestMakeIngressRule_NilPercentTarget(t *testing.T) {
 		Visibility: netv1alpha1.IngressVisibilityExternalIP,
 	}
 
-	if !cmp.Equal(&expected, rule) {
-		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(&expected, rule))
+	if !cmp.Equal(expected, rule) {
+		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(expected, rule))
 	}
 }
 
@@ -665,8 +665,8 @@ func TestMakeIngressRule_TwoTargets(t *testing.T) {
 		Visibility: netv1alpha1.IngressVisibilityExternalIP,
 	}
 
-	if !cmp.Equal(&expected, rule) {
-		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(&expected, rule))
+	if !cmp.Equal(expected, rule) {
+		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(expected, rule))
 	}
 }
 
@@ -706,8 +706,8 @@ func TestMakeIngressRule_InactiveTarget(t *testing.T) {
 		},
 		Visibility: netv1alpha1.IngressVisibilityExternalIP,
 	}
-	if !cmp.Equal(&expected, rule) {
-		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(&expected, rule))
+	if !cmp.Equal(expected, rule) {
+		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(expected, rule))
 	}
 }
 
@@ -766,8 +766,8 @@ func TestMakeIngressRule_TwoInactiveTargets(t *testing.T) {
 		},
 		Visibility: netv1alpha1.IngressVisibilityExternalIP,
 	}
-	if !cmp.Equal(&expected, rule) {
-		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(&expected, rule))
+	if !cmp.Equal(expected, rule) {
+		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(expected, rule))
 	}
 }
 
@@ -811,8 +811,8 @@ func TestMakeIngressRule_ZeroPercentTargetInactive(t *testing.T) {
 		Visibility: netv1alpha1.IngressVisibilityExternalIP,
 	}
 
-	if !cmp.Equal(&expected, rule) {
-		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(&expected, rule))
+	if !cmp.Equal(expected, rule) {
+		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(expected, rule))
 	}
 }
 
@@ -856,12 +856,12 @@ func TestMakeIngressRule_NilPercentTargetInactive(t *testing.T) {
 		Visibility: netv1alpha1.IngressVisibilityExternalIP,
 	}
 
-	if !cmp.Equal(&expected, rule) {
-		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(&expected, rule))
+	if !cmp.Equal(expected, rule) {
+		t.Errorf("Unexpected rule (-want, +got): %s", cmp.Diff(expected, rule))
 	}
 }
 
-func TestMakeIngress_WithTLS(t *testing.T) {
+func TestMakeIngressWithTLS(t *testing.T) {
 	targets := map[string]traffic.RevisionTargets{}
 	ingressClass := "foo-ingress"
 	r := Route(ns, "test-route", WithRouteUID("1234-5678"), WithURL)
@@ -920,7 +920,7 @@ func TestMakeIngressTLS(t *testing.T) {
 	}
 }
 
-func TestMakeIngress_ACMEChallenges(t *testing.T) {
+func TestMakeIngressACMEChallenges(t *testing.T) {
 	targets := map[string]traffic.RevisionTargets{
 		traffic.DefaultTarget: {{
 			TrafficTarget: v1.TrafficTarget{
@@ -1022,7 +1022,7 @@ func TestMakeIngress_ACMEChallenges(t *testing.T) {
 
 }
 
-func TestMakeIngress_FailToGenerateDomain(t *testing.T) {
+func TestMakeIngressFailToGenerateDomain(t *testing.T) {
 	targets := map[string]traffic.RevisionTargets{
 		traffic.DefaultTarget: {{
 			TrafficTarget: v1.TrafficTarget{
@@ -1055,7 +1055,7 @@ func TestMakeIngress_FailToGenerateDomain(t *testing.T) {
 	}
 }
 
-func TestMakeIngress_FailToGenerateTagHost(t *testing.T) {
+func TestMakeIngressFailToGenerateTagHost(t *testing.T) {
 	targets := map[string]traffic.RevisionTargets{
 		traffic.DefaultTarget: {{
 			TrafficTarget: v1.TrafficTarget{
diff --git a/pkg/reconciler/route/resources/service_test.go b/pkg/reconciler/route/resources/service_test.go
index f98e8e056..ece13e115 100644
--- a/pkg/reconciler/route/resources/service_test.go
+++ b/pkg/reconciler/route/resources/service_test.go
@@ -22,10 +22,12 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
+
 	netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
 	"knative.dev/pkg/kmeta"
 	"knative.dev/serving/pkg/apis/serving"