Repository: knative/pkg
Pull request #1595 (2020-08-07 22:11:03)

Switch to the atomic bool from uber package for the profiling flag
- makes code easier to follow
- hides the int32->bool conversions inside the library

/assign mattmoor

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

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

- ~~[OWNERS](https://github.com/knative/pkg/blob/master/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":[]} -->
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
---- |:------------:|:------------:|:-----:
[profiling/server.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1595/pull-knative-pkg-go-coverage/1291859622243602435/artifacts/line-cov.html#file0) | 93.5% | 92.3% | -1.2

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

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 15df0e117208e23f3253a2452157a4e0d5914ec1 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1595/pull-knative-pkg-unit-tests/1291859622243602434) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
vagababov: /retest
knative-prow-robot: @vagababov: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 15df0e117208e23f3253a2452157a4e0d5914ec1 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1595/pull-knative-pkg-unit-tests/1291864276499173376) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
vagababov: /retest
knative-prow-robot: @vagababov: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 15df0e117208e23f3253a2452157a4e0d5914ec1 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1595/pull-knative-pkg-unit-tests/1291889442881015812) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
vagababov: /lgtm

knative-prow-robot: @vagababov: you cannot LGTM your own PR.

<details>

In response to [this](https://github.com/knative/pkg/pull/1595#issuecomment-670794048):

>/lgtm
>


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.
</details>
vagababov: FFS
/retest
knative-prow-robot: @vagababov: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 15df0e117208e23f3253a2452157a4e0d5914ec1 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1595/pull-knative-pkg-unit-tests/1291923164770603009) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
vagababov: /retest
knative-prow-robot: @vagababov: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 15df0e117208e23f3253a2452157a4e0d5914ec1 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1595/pull-knative-pkg-unit-tests/1291962171793608704) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
vagababov: /retet
vagababov: /retest

Patch:
diff --git a/go.mod b/go.mod
index 303f69d46..cdaa4b77e 100644
--- a/go.mod
+++ b/go.mod
@@ -32,6 +32,7 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/tsenart/vegeta v12.7.1-0.20190725001342-b5f4fca92137+incompatible
 	go.opencensus.io v0.22.4
+	go.uber.org/atomic v1.6.0
 	go.uber.org/multierr v1.5.0
 	go.uber.org/zap v1.15.0
 	golang.org/x/net v0.0.0-20200707034311-ab3426394381
@@ -50,7 +51,7 @@ require (
 	k8s.io/code-generator v0.18.6
 	k8s.io/gengo v0.0.0-20200205140755-e0e292d8aa12
 	k8s.io/klog v1.0.0
-	knative.dev/test-infra v0.0.0-20200803175002-5efff0c4bd0a
+	knative.dev/test-infra v0.0.0-20200806191129-68b7defbd189
 	sigs.k8s.io/boskos v0.0.0-20200729174948-794df80db9c9
 )
 
diff --git a/go.sum b/go.sum
index d0ca59661..6c06000ff 100644
--- a/go.sum
+++ b/go.sum
@@ -1834,8 +1834,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-20200803175002-5efff0c4bd0a h1:0woae+DQoLaB8PBL1V2LKoPVOm3rldG4G1HYGROo1bo=
-knative.dev/test-infra v0.0.0-20200803175002-5efff0c4bd0a/go.mod h1:Pmg2c7Z7q7BGFUV/GOpU5BlrD3ePJft4MPqx8AYBplc=
+knative.dev/test-infra v0.0.0-20200806191129-68b7defbd189 h1:PkTuK9kgtsdwrIxUAmaCmDwfRKBoXujiqNRDWO1r8cc=
+knative.dev/test-infra v0.0.0-20200806191129-68b7defbd189/go.mod h1:Pmg2c7Z7q7BGFUV/GOpU5BlrD3ePJft4MPqx8AYBplc=
 modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw=
 modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
 modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
diff --git a/profiling/server.go b/profiling/server.go
index fe27ac04e..e543489ad 100644
--- a/profiling/server.go
+++ b/profiling/server.go
@@ -21,8 +21,8 @@ import (
 	"net/http"
 	"net/http/pprof"
 	"strconv"
-	"sync/atomic"
 
+	"go.uber.org/atomic"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 )
@@ -39,7 +39,7 @@ const (
 // Handler holds the main HTTP handler and a flag indicating
 // whether the handler is active
 type Handler struct {
-	enabled int32
+	enabled *atomic.Bool
 	handler http.Handler
 	log     *zap.SugaredLogger
 }
@@ -58,14 +58,14 @@ func NewHandler(logger *zap.SugaredLogger, enableProfiling bool) *Handler {
 
 	logger.Infof("Profiling enabled: %t", enableProfiling)
 	return &Handler{
-		enabled: boolToInt32(enableProfiling),
+		enabled: atomic.NewBool(enableProfiling),
 		handler: mux,
 		log:     logger,
 	}
 }
 
 func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	if atomic.LoadInt32(&h.enabled) == 1 {
+	if h.enabled.Load() {
 		h.handler.ServeHTTP(w, r)
 	} else {
 		http.NotFoundHandler().ServeHTTP(w, r)
@@ -93,9 +93,7 @@ func (h *Handler) UpdateFromConfigMap(configMap *corev1.ConfigMap) {
 		return
 	}
 
-	new := boolToInt32(enabled)
-	old := atomic.SwapInt32(&h.enabled, new)
-	if old != new {
+	if h.enabled.Swap(enabled) != enabled {
 		h.log.Infof("Profiling enabled: %t", enabled)
 	}
 }
@@ -107,10 +105,3 @@ func NewServer(handler http.Handler) *http.Server {
 		Handler: handler,
 	}
 }
-
-func boolToInt32(b bool) int32 {
-	if b {
-		return 1
-	}
-	return 0
-}
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 4d73c007d..f4538df07 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -286,6 +286,7 @@ go.opencensus.io/trace/internal
 go.opencensus.io/trace/propagation
 go.opencensus.io/trace/tracestate
 # go.uber.org/atomic v1.6.0
+## explicit
 go.uber.org/atomic
 # go.uber.org/multierr v1.5.0
 ## explicit
@@ -945,7 +946,7 @@ k8s.io/utils/buffer
 k8s.io/utils/integer
 k8s.io/utils/pointer
 k8s.io/utils/trace
-# knative.dev/test-infra v0.0.0-20200803175002-5efff0c4bd0a
+# knative.dev/test-infra v0.0.0-20200806191129-68b7defbd189
 ## explicit
 knative.dev/test-infra/scripts
 # sigs.k8s.io/boskos v0.0.0-20200729174948-794df80db9c9


Test patch:
diff --git a/profiling/server_test.go b/profiling/server_test.go
index f2213895a..b19f94516 100644
--- a/profiling/server_test.go
+++ b/profiling/server_test.go
@@ -19,7 +19,6 @@ package profiling
 import (
 	"net/http"
 	"net/http/httptest"
-	"sync/atomic"
 	"testing"
 
 	"go.uber.org/zap"
@@ -96,8 +95,8 @@ func TestUpdateFromConfigMap(t *testing.T) {
 				t.Errorf("StatusCode: %v, want: %v", rr.Code, tt.wantStatusCode)
 			}
 
-			if atomic.LoadInt32(&handler.enabled) != boolToInt32(tt.wantEnabled) {
-				t.Fatalf("Test: %q; want %v, but got %v", tt.name, tt.wantEnabled, handler.enabled)
+			if handler.enabled.Load() != tt.wantEnabled {
+				t.Fatalf("Enabled got %v, want %v", handler.enabled.Load(), tt.wantEnabled)
 			}
 		})
 	}