Repository: knative/pkg
Pull request #1720 (2020-09-19 04:23:43)

Deep copy views to avoid races

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1720#" title="Author self-approved">ian-mi</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **mdemirhan**
You can assign the PR to them by writing `/assign @mdemirhan` 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:

- **[metrics/OWNERS](https://github.com/knative/pkg/blob/master/metrics/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":["mdemirhan"]} -->
knative-prow-robot: @ian-mi: The following test **failed**, say `/retest` to rerun all failed tests:

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

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1720). [Your PR dashboard](https://gubernator.knative.dev/pr/ian-mi).

<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 -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1720?src=pr&el=h1) Report
> Merging [#1720](https://codecov.io/gh/knative/pkg/pull/1720?src=pr&el=desc) into [master](https://codecov.io/gh/knative/pkg/commit/8c2ebdb0e0fcc3fadfd4ca359b62e2e4a72fa1d5?el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.

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

```diff
@@            Coverage Diff             @@
##           master    #1720      +/-   ##
==========================================
- Coverage   67.62%   67.62%   -0.01%     
==========================================
  Files         226      226              
  Lines        9545     9551       +6     
==========================================
+ Hits         6455     6459       +4     
- Misses       2719     2720       +1     
- Partials      371      372       +1     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1720?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [metrics/resource\_view.go](https://codecov.io/gh/knative/pkg/pull/1720/diff?src=pr&el=tree#diff-bWV0cmljcy9yZXNvdXJjZV92aWV3Lmdv) | `84.55% <100.00%> (+0.71%)` | :arrow_up: |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/1720/diff?src=pr&el=tree#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `90.32% <0.00%> (-1.08%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1720?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/1720?src=pr&el=footer). Last update [8c2ebdb...8cd9aa6](https://codecov.io/gh/knative/pkg/pull/1720?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[metrics/resource_view.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1720/pull-knative-pkg-go-coverage/1307192102270013442/artifacts/line-cov.html#file0) | 91.5% | 91.8% | 0.3


Patch:
diff --git a/.github/workflows/knative-donotsubmit.yaml b/.github/workflows/knative-donotsubmit.yaml
index dcfd1ed86..92a96636b 100644
--- a/.github/workflows/knative-donotsubmit.yaml
+++ b/.github/workflows/knative-donotsubmit.yaml
@@ -49,10 +49,7 @@ jobs:
           echo '::group:: Running DO NOT SUBMIT with reviewdog 🐶 ...'
           # Don't fail because of grep
           set +o pipefail
-          find . -path './vendor' -prune \
-                 -o -path './third_party' -prune \
-                 -o -path './.github/workflows' -prune \
-                 -o -type f |
+          find . -type f -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*' -not -path './.github/workflows/*' |
           xargs grep -n "DO NOT SUBMIT" |
           reviewdog -efm="%f:%l:%m" \
                 -name="DO NOT SUBMIT" \
diff --git a/.github/workflows/knative-style.yaml b/.github/workflows/knative-style.yaml
index 687f4b195..49ef66bf1 100644
--- a/.github/workflows/knative-style.yaml
+++ b/.github/workflows/knative-style.yaml
@@ -38,20 +38,42 @@ jobs:
       - name: Check out code
         uses: actions/checkout@v2
 
+      - name: Install Dependencies
+        run: |
+          cd $(mktemp -d)
+          GO111MODULE=on go get golang.org/x/tools/cmd/goimports
+
       # Run this last because it alters the workspace.
       # TODO: add prettier step
-      # TODO: add goimports step
+      - name: Go Imports
+        shell: bash
+        run: |
+          goimports -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
+          echo "::set-env name=FMT_TOOL::goimports"
+      - name: Verify goimport
+        shell: bash
+        run: |
+          if [[ $(git diff-index --name-only HEAD --) ]]; then
+              echo "Found diffs in:"
+              git diff-index --name-only HEAD --
+              echo "${{ github.repository }} is out of style. Please run $FMT_TOOL."
+              exit 1
+          fi
+          echo "${{ github.repository }} is formatted correctly."
+
       - name: Go Format
         shell: bash
         run: |
-          gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -type f -name '*.go' -print)
-      - name: Verify
+          gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
+          echo "::set-env name=FMT_TOOL::gofmt"
+      - name: Verify gofmt
         shell: bash
+        # TODO(mattmoor): combine with above via anchors when actions support it.
         run: |
           if [[ $(git diff-index --name-only HEAD --) ]]; then
               echo "Found diffs in:"
               git diff-index --name-only HEAD --
-              echo "${{ github.repository }} is out of style. Please run go fmt."
+              echo "${{ github.repository }} is out of style. Please run $FMT_TOOL."
               exit 1
           fi
           echo "${{ github.repository }} is formatted correctly."
@@ -71,11 +93,29 @@ jobs:
       - name: Check out code
         uses: actions/checkout@v2
 
+      - name: Install Tools
+        run: |
+          TEMP_PATH="$(mktemp -d)"
+          cd $TEMP_PATH
+
+          echo '::group::🐶 Installing reviewdog ... https://github.com/reviewdog/reviewdog'
+          curl -sfL https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b "${TEMP_PATH}" 2>&1
+          echo '::endgroup::'
+
+          echo '::group:: Installing misspell ... https://github.com/client9/misspell'
+          go get github.com/client9/misspell/cmd/misspell
+          echo '::endgroup::'
+
+          echo '::group:: Installing woke ... https://github.com/get-woke/woke'
+          curl -sfL https://raw.githubusercontent.com/get-woke/woke/main/install.sh | sh -s -- -b "${TEMP_PATH}" "${WOKE_VERSION}" 2>&1
+          echo '::endgroup::'
+
+          echo "::add-path::${TEMP_PATH}"
+
       - id: golangci_configuration
         uses: andstor/file-existence-action@v1
         with:
           files: .golangci.yaml
-
       - name: Go Lint
         if: steps.golangci_configuration.outputs.files_exists == 'true'
         uses: golangci/golangci-lint-action@v2
@@ -92,26 +132,65 @@ jobs:
           set -e
           cd "${GITHUB_WORKSPACE}" || exit 1
 
-          TEMP_PATH="$(mktemp -d)"
-          PATH="${TEMP_PATH}:$PATH"
+          echo '::group:: Running github.com/client9/misspell with reviewdog 🐶 ...'
+          # Don't fail because of misspell
+          set +o pipefail
+          find . -type f -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*' |
+          xargs misspell -error |
+          reviewdog -efm="%f:%l:%c: %m" \
+                -name="github.com/client9/misspell" \
+                -reporter="github-pr-check" \
+                -filter-mode="added" \
+                -fail-on-error="true" \
+                -level="error"
 
-          echo '::group:: Installing misspell ... https://github.com/client9/misspell'
-          go get github.com/client9/misspell/cmd/misspell
           echo '::endgroup::'
 
-          echo '::group::🐶 Installing reviewdog ... https://github.com/reviewdog/reviewdog'
-          curl -sfL https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b "${TEMP_PATH}" 2>&1
+      - name: trailing whitespace
+        shell: bash
+        if: ${{ always() }}
+        env:
+          REVIEWDOG_GITHUB_API_TOKEN: ${{ github.token }}
+        run: |
+          set -e
+          cd "${GITHUB_WORKSPACE}" || exit 1
+
+          echo '::group:: Flagging trailing whitespace with reviewdog 🐶 ...'
+          # Don't fail because of grep
+          set +o pipefail
+          find . -type f -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*' |
+          xargs grep -nE " +$" |
+          reviewdog -efm="%f:%l:%m" \
+                -name="trailing whitespace" \
+                -reporter="github-pr-check" \
+                -filter-mode="added" \
+                -fail-on-error="true" \
+                -level="error"
+
           echo '::endgroup::'
 
-          echo '::group:: Running github.com/client9/misspell with reviewdog 🐶 ...'
+      - name: EOF newline
+        shell: bash
+        if: ${{ always() }}
+        env:
+          REVIEWDOG_GITHUB_API_TOKEN: ${{ github.token }}
+        run: |
+          set -e
+          cd "${GITHUB_WORKSPACE}" || exit 1
+
+          echo '::group:: Flagging missing EOF newlines with reviewdog 🐶 ...'
           # Don't fail because of misspell
           set +o pipefail
-          find . -path './vendor' -prune \
-                 -o -path './third_party' -prune \
-                 -o -type f |
-          xargs misspell -error |
-          reviewdog -efm="%f:%l:%c: %m" \
-                -name="github.com/client9/misspell" \
+          for x in $(find . -type f -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*'); do
+            # Based on https://stackoverflow.com/questions/34943632/linux-check-if-there-is-an-empty-line-at-the-end-of-a-file
+            if [[ -f $x && ! ( -s "$x" && -z "$(tail -c 1 $x)" ) ]]; then
+              # We add 1 to `wc -l` here because of this limitation (from the man page):
+              # Characters beyond the final <newline> character will not be included in the line count.
+              echo $x:$((1 + $(wc -l $x | tr -s ' ' | cut -d' ' -f 1))): Missing newline
+            fi
+          done |
+          reviewdog -efm="%f:%l: %m" \
+                -name="EOF Newline" \
                 -reporter="github-pr-check" \
                 -filter-mode="added" \
                 -fail-on-error="true" \
@@ -131,20 +210,12 @@ jobs:
           set -e
           cd "${GITHUB_WORKSPACE}" || exit 1
 
-          TEMP_PATH="$(mktemp -d)"
-          PATH="${TEMP_PATH}:$PATH"
-
-          echo '::group::🐶 Installing reviewdog ... https://github.com/reviewdog/reviewdog'
-          curl -sfL https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b "${TEMP_PATH}" 2>&1
-          echo '::endgroup::'
-
-          echo '::group:: Installing woke ... https://github.com/get-woke/woke'
-          curl -sfL https://raw.githubusercontent.com/get-woke/woke/main/install.sh | sh -s -- -b "${TEMP_PATH}" "${WOKE_VERSION}" 2>&1
-          echo '::endgroup::'
-
           # Create a minimal .wokeignore if none already exist.
           if [ ! -f .wokeignore ]; then
-            echo "vendor\nthird_party" > .wokeignore
+            cat > .wokeignore <<EOF
+            vendor/*
+            third_party/*
+          EOF
           fi
 
           echo '::group:: Running woke with reviewdog 🐶 ...'
diff --git a/go.mod b/go.mod
index 77197770c..e1224ed17 100644
--- a/go.mod
+++ b/go.mod
@@ -55,7 +55,7 @@ require (
 	k8s.io/code-generator v0.18.8
 	k8s.io/gengo v0.0.0-20200205140755-e0e292d8aa12
 	k8s.io/klog v1.0.0
-	knative.dev/test-infra v0.0.0-20200918021244-22269d552166
+	knative.dev/test-infra v0.0.0-20200921012245-37f1a12adbd3
 	sigs.k8s.io/boskos v0.0.0-20200729174948-794df80db9c9
 )
 
diff --git a/go.sum b/go.sum
index 4fbd3c6e8..1e2569a19 100644
--- a/go.sum
+++ b/go.sum
@@ -1846,8 +1846,8 @@ knative.dev/test-infra v0.0.0-20200505052144-5ea2f705bb55/go.mod h1:WqF1Azka+FxP
 knative.dev/test-infra v0.0.0-20200513011557-d03429a76034/go.mod h1:aMif0KXL4g19YCYwsy4Ocjjz5xgPlseYV+B95Oo4JGE=
 knative.dev/test-infra v0.0.0-20200519015156-82551620b0a9/go.mod h1:A5b2OAXTOeHT3hHhVQm3dmtbuWvIDP7qzgtqxA3/2pE=
 knative.dev/test-infra v0.0.0-20200707183444-aed09e56ddc7/go.mod h1:RjYAhXnZqeHw9+B0zsbqSPlae0lCvjekO/nw5ZMpLCs=
-knative.dev/test-infra v0.0.0-20200918021244-22269d552166 h1:7Ls02/98BkpQbCItaIjkpNDKGy5oOam1SsGS6U0zl54=
-knative.dev/test-infra v0.0.0-20200918021244-22269d552166/go.mod h1:Pmg2c7Z7q7BGFUV/GOpU5BlrD3ePJft4MPqx8AYBplc=
+knative.dev/test-infra v0.0.0-20200921012245-37f1a12adbd3 h1:abacWewpeP9QUByQAunuOA8F9hQ6Ryr2P63r38ns5sA=
+knative.dev/test-infra v0.0.0-20200921012245-37f1a12adbd3/go.mod h1:Pmg2c7Z7q7BGFUV/GOpU5BlrD3ePJft4MPqx8AYBplc=
 mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed/go.mod h1:Xkxe497xwlCKkIaQYRfC7CSLworTXY9RMqwhhCm+8Nc=
 mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b/go.mod h1:2odslEg/xrtNQqCYg2/jCoyKnw3vv5biOc3JnIcYfL4=
 mvdan.cc/unparam v0.0.0-20190720180237-d51796306d8f/go.mod h1:4G1h5nDURzA3bwVMZIVpwbkw+04kSxk3rAtzlimaUJw=
diff --git a/metrics/resource_view.go b/metrics/resource_view.go
index 7d69ae94e..1e89787f6 100644
--- a/metrics/resource_view.go
+++ b/metrics/resource_view.go
@@ -72,11 +72,7 @@ func RegisterResourceView(views ...*view.View) error {
 	defer resourceViews.lock.Unlock()
 	for _, meter := range allMeters.meters {
 		// make a copy of views to avoid data races
-		viewCopy := make([]*view.View, 0, len(views))
-		for _, v := range views {
-			c := *v
-			viewCopy = append(viewCopy, &c)
-		}
+		viewCopy := copyViews(views)
 		if e := meter.m.Register(viewCopy...); e != nil {
 			err = e
 		}
@@ -213,11 +209,7 @@ func meterExporterForResource(r *resource.Resource) *meterExporter {
 	resourceViews.lock.Lock()
 	defer resourceViews.lock.Unlock()
 	// make a copy of views to avoid data races
-	viewsCopy := make([]*view.View, 0, len(resourceViews.views))
-	for _, v := range resourceViews.views {
-		c := *v
-		viewsCopy = append(viewsCopy, &c)
-	}
+	viewsCopy := copyViews(resourceViews.views)
 	mE.m.Register(viewsCopy...)
 	mE.o = stats.WithRecorder(mE.m)
 	allMeters.meters[key] = mE
@@ -295,6 +287,21 @@ func resourceFromKey(s string) *resource.Resource {
 	return r
 }
 
+func copyViews(views []*view.View) []*view.View {
+	viewsCopy := make([]*view.View, 0, len(resourceViews.views))
+	for _, v := range views {
+		c := *v
+		c.TagKeys = make([]tag.Key, len(v.TagKeys))
+		copy(c.TagKeys, v.TagKeys)
+		agg := *v.Aggregation
+		c.Aggregation = &agg
+		c.Aggregation.Buckets = make([]float64, len(v.Aggregation.Buckets))
+		copy(c.Aggregation.Buckets, v.Aggregation.Buckets)
+		viewsCopy = append(viewsCopy, &c)
+	}
+	return viewsCopy
+}
+
 // defaultMeterImpl is a pass-through to the default worker in OpenCensus.
 type defaultMeterImpl struct{}
 
diff --git a/network/domain.go b/network/domain.go
index bc2ef6a99..a205b4eec 100644
--- a/network/domain.go
+++ b/network/domain.go
@@ -31,12 +31,12 @@ const (
 )
 
 var (
-	domainName string
+	domainName = defaultDomainName
 	once       sync.Once
 )
 
 // GetServiceHostname returns the fully qualified service hostname
-func GetServiceHostname(name string, namespace string) string {
+func GetServiceHostname(name, namespace string) string {
 	return fmt.Sprintf("%s.%s.svc.%s", name, namespace, GetClusterDomainName())
 }
 
@@ -45,31 +45,27 @@ func GetServiceHostname(name string, namespace string) string {
 func GetClusterDomainName() string {
 	once.Do(func() {
 		f, err := os.Open(resolverFileName)
-		if err == nil {
-			defer f.Close()
-			domainName = getClusterDomainName(f)
-
-		} else {
-			domainName = defaultDomainName
+		if err != nil {
+			return
 		}
+		defer f.Close()
+		domainName = getClusterDomainName(f)
 	})
-
 	return domainName
 }
 
 func getClusterDomainName(r io.Reader) string {
-	scanner := bufio.NewScanner(r)
-	for scanner.Scan() {
+	for scanner := bufio.NewScanner(r); scanner.Scan(); {
 		elements := strings.Split(scanner.Text(), " ")
 		if elements[0] != "search" {
 			continue
 		}
-		for i := 1; i < len(elements)-1; i++ {
-			if strings.HasPrefix(elements[i], "svc.") {
-				return strings.TrimSuffix(elements[i][4:], ".")
+		for _, e := range elements[1:] {
+			if strings.HasPrefix(e, "svc.") {
+				return strings.TrimSuffix(e[4:], ".")
 			}
 		}
 	}
-	// For all abnormal cases return default domain name
+	// For all abnormal cases return default domain name.
 	return defaultDomainName
 }
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 36ddc75ff..906ba91f1 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -975,7 +975,7 @@ k8s.io/utils/buffer
 k8s.io/utils/integer
 k8s.io/utils/pointer
 k8s.io/utils/trace
-# knative.dev/test-infra v0.0.0-20200918021244-22269d552166
+# knative.dev/test-infra v0.0.0-20200921012245-37f1a12adbd3
 ## explicit
 knative.dev/test-infra/scripts
 # sigs.k8s.io/boskos v0.0.0-20200729174948-794df80db9c9
diff --git a/webhook/stats_reporter.go b/webhook/stats_reporter.go
index 13293200c..4c6d72134 100644
--- a/webhook/stats_reporter.go
+++ b/webhook/stats_reporter.go
@@ -55,7 +55,6 @@ var (
 	resourceGroupKey     = tag.MustNewKey("resource_group")
 	resourceVersionKey   = tag.MustNewKey("resource_version")
 	resourceResourceKey  = tag.MustNewKey("resource_resource")
-	resourceNameKey      = tag.MustNewKey("resource_name")
 	resourceNamespaceKey = tag.MustNewKey("resource_namespace")
 	admissionAllowedKey  = tag.MustNewKey("admission_allowed")
 )
@@ -93,7 +92,6 @@ func (r *reporter) ReportRequest(req *admissionv1.AdmissionRequest, resp *admiss
 		tag.Insert(resourceGroupKey, req.Resource.Group),
 		tag.Insert(resourceVersionKey, req.Resource.Version),
 		tag.Insert(resourceResourceKey, req.Resource.Resource),
-		tag.Insert(resourceNameKey, req.Name),
 		tag.Insert(resourceNamespaceKey, req.Namespace),
 		tag.Insert(admissionAllowedKey, strconv.FormatBool(resp.Allowed)),
 	)
@@ -117,7 +115,6 @@ func RegisterMetrics() {
 		resourceVersionKey,
 		resourceResourceKey,
 		resourceNamespaceKey,
-		resourceNameKey,
 		admissionAllowedKey}
 
 	if err := view.Register(


Test patch:
diff --git a/test/clients.go b/test/clients.go
index fbff159ef..bd1e58a6b 100644
--- a/test/clients.go
+++ b/test/clients.go
@@ -39,8 +39,10 @@ type KubeClient struct {
 }
 
 // NewSpoofingClient returns a spoofing client to make requests
-func NewSpoofingClient(ctx context.Context, client *KubeClient, logf logging.FormatLogger, domain string, resolvable bool, opts ...spoof.TransportOption) (*spoof.SpoofingClient, error) {
-	return spoof.New(ctx, client.Kube, logf, domain, resolvable, Flags.IngressEndpoint, Flags.SpoofRequestInterval, Flags.SpoofRequestTimeout, opts...)
+func NewSpoofingClient(ctx context.Context, client *KubeClient, logf logging.FormatLogger,
+	domain string, resolvable bool, opts ...spoof.TransportOption) (*spoof.SpoofingClient, error) {
+	return spoof.New(ctx, client.Kube, logf, domain, resolvable, Flags.IngressEndpoint,
+		Flags.SpoofRequestInterval, Flags.SpoofRequestTimeout, opts...)
 }
 
 // NewKubeClient instantiates and returns several clientsets required for making request to the
diff --git a/test/request.go b/test/request.go
index 20fd73c11..b26eb0c2a 100644
--- a/test/request.go
+++ b/test/request.go
@@ -147,8 +147,7 @@ func EventuallyMatchesBody(expected string) spoof.ResponseChecker {
 func MatchesAllOf(checkers ...spoof.ResponseChecker) spoof.ResponseChecker {
 	return func(resp *spoof.Response) (bool, error) {
 		for _, checker := range checkers {
-			done, err := checker(resp)
-			if err != nil || !done {
+			if done, err := checker(resp); err != nil || !done {
 				return done, err
 			}
 		}
@@ -171,7 +170,8 @@ func WaitForEndpointState(
 	desc string,
 	resolvable bool,
 	opts ...interface{}) (*spoof.Response, error) {
-	return WaitForEndpointStateWithTimeout(ctx, kubeClient, logf, url, inState, desc, resolvable, Flags.SpoofRequestTimeout, opts...)
+	return WaitForEndpointStateWithTimeout(ctx, kubeClient, logf, url, inState,
+		desc, resolvable, Flags.SpoofRequestTimeout, opts...)
 }
 
 // WaitForEndpointStateWithTimeout will poll an endpoint until inState indicates the state is achieved
@@ -190,7 +190,7 @@ func WaitForEndpointStateWithTimeout(
 	resolvable bool,
 	timeout time.Duration,
 	opts ...interface{}) (*spoof.Response, error) {
-	defer logging.GetEmitableSpan(ctx, fmt.Sprintf("WaitForEndpointState/%s", desc)).End()
+	defer logging.GetEmitableSpan(ctx, "WaitForEndpointState/"+desc).End()
 
 	if url.Scheme == "" || url.Host == "" {
 		return nil, fmt.Errorf("invalid URL: %q", url.String())
diff --git a/test/spoof/spoof.go b/test/spoof/spoof.go
index 3278ca346..06194090f 100644
--- a/test/spoof/spoof.go
+++ b/test/spoof/spoof.go
@@ -39,12 +39,6 @@ import (
 	"go.opencensus.io/trace"
 )
 
-const (
-	// Name of the temporary HTTP header that is added to http.Request to indicate that
-	// it is a SpoofClient.Poll request. This header is removed before making call to backend.
-	pollReqHeader = "X-Kn-Poll-Request-Do-Not-Trace"
-)
-
 // Response is a stripped down subset of http.Response. The is primarily useful
 // for ResponseCheckers to inspect the response body without consuming it.
 // Notably, Body is a byte slice instead of an io.ReadCloser.
diff --git a/vendor/knative.dev/test-infra/scripts/library.sh b/vendor/knative.dev/test-infra/scripts/library.sh
index e7ecbc9be..ab9f303fe 100644
--- a/vendor/knative.dev/test-infra/scripts/library.sh
+++ b/vendor/knative.dev/test-infra/scripts/library.sh
@@ -127,13 +127,50 @@ function wait_until_object_does_not_exist() {
 # Waits until all pods are running in the given namespace.
 # Parameters: $1 - namespace.
 function wait_until_pods_running() {
-  echo "Waiting until all pods in namespace $1 are up"
-  kubectl wait pod --for=condition=Ready -n "$1" -l '!job-name' --timeout=5m || return 1
-  # Also wait for all the job pods to be completed.
-  # This is mainly for maintaining backward compatibility.
-  if [[ $(kubectl get jobs --ignore-not-found=true -n "$1") ]]; then
-    kubectl wait job --for=condition=Complete --all -n "$1" --timeout=5m || return 1
+  echo -n "Waiting until all pods in namespace $1 are up"
+  local failed_pod=""
+  for i in {1..150}; do  # timeout after 5 minutes
+    # List all pods. Ignore Terminating pods as those have either been replaced through
+    # a deployment or terminated on purpose (through chaosduck for example).
+    local pods="$(kubectl get pods --no-headers -n $1 2>/dev/null | grep -v Terminating)"
+    # All pods must be running (ignore ImagePull error to allow the pod to retry)
+    local not_running_pods=$(echo "${pods}" | grep -v Running | grep -v Completed | grep -v ErrImagePull | grep -v ImagePullBackOff)
+    if [[ -n "${pods}" ]] && [[ -z "${not_running_pods}" ]]; then
+      # All Pods are running or completed. Verify the containers on each Pod.
+      local all_ready=1
+      while read pod ; do
+        local status=(`echo -n ${pod} | cut -f2 -d' ' | tr '/' ' '`)
+        # Set this Pod as the failed_pod. If nothing is wrong with it, then after the checks, set
+        # failed_pod to the empty string.
+        failed_pod=$(echo -n "${pod}" | cut -f1 -d' ')
+        # All containers must be ready
+        [[ -z ${status[0]} ]] && all_ready=0 && break
+        [[ -z ${status[1]} ]] && all_ready=0 && break
+        [[ ${status[0]} -lt 1 ]] && all_ready=0 && break
+        [[ ${status[1]} -lt 1 ]] && all_ready=0 && break
+        [[ ${status[0]} -ne ${status[1]} ]] && all_ready=0 && break
+        # All the tests passed, this is not a failed pod.
+        failed_pod=""
+      done <<< "$(echo "${pods}" | grep -v Completed)"
+      if (( all_ready )); then
+        echo -e "\nAll pods are up:\n${pods}"
+        return 0
+      fi
+    elif [[ -n "${not_running_pods}" ]]; then
+      # At least one Pod is not running, just save the first one's name as the failed_pod.
+      failed_pod="$(echo "${not_running_pods}" | head -n 1 | cut -f1 -d' ')"
+    fi
+    echo -n "."
+    sleep 2
+  done
+  echo -e "\n\nERROR: timeout waiting for pods to come up\n${pods}"
+  if [[ -n "${failed_pod}" ]]; then
+    echo -e "\n\nFailed Pod (data in YAML format) - ${failed_pod}\n"
+    kubectl -n $1 get pods "${failed_pod}" -oyaml
+    echo -e "\n\nPod Logs\n"
+    kubectl -n $1 logs "${failed_pod}" --all-containers
   fi
+  return 1
 }
 
 # Waits until all batch jobs complete in the given namespace.
diff --git a/webhook/stats_reporter_test.go b/webhook/stats_reporter_test.go
index 94b1cc47c..755c14cfb 100644
--- a/webhook/stats_reporter_test.go
+++ b/webhook/stats_reporter_test.go
@@ -54,7 +54,6 @@ func TestWebhookStatsReporter(t *testing.T) {
 		resourceGroupKey.Name():     req.Resource.Group,
 		resourceVersionKey.Name():   req.Resource.Version,
 		resourceResourceKey.Name():  req.Resource.Resource,
-		resourceNameKey.Name():      req.Name,
 		resourceNamespaceKey.Name(): req.Namespace,
 		admissionAllowedKey.Name():  strconv.FormatBool(resp.Allowed),
 	}