Repository: knative/pkg
Pull request #1688 (2020-09-08 20:22:13)

Alter URIFromObjectReference to return apierrs
This allows for reconcile deletion to properly remove finalizer and
delete sinkbinding

Followup from #1647

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

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

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

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

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

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

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

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

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 4158d09fd9064c8a36c1b855259932ea8f3c6d92 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1688/pull-knative-pkg-unit-tests/1303430205431353345) | `/test pull-knative-pkg-unit-tests`

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

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: @lberk: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 3657ae0241d9366ae7c6fc8e5d63a8d5cc1bcf9f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1688/pull-knative-pkg-unit-tests/1303436749220876289) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
lberk: /test pull-knative-pkg-unit-tests	

Patch:
diff --git a/.golangci.yaml b/.golangci.yaml
new file mode 100644
index 000000000..a33656679
--- /dev/null
+++ b/.golangci.yaml
@@ -0,0 +1,13 @@
+run:
+  timeout: 5m
+
+  skip-dirs:
+    - injection/client
+    - test/mako
+
+linters:
+  enable:
+    - unconvert
+    - prealloc
+  disable:
+    - errcheck
diff --git a/apiextensions/storageversion/cmd/migrate/config.go b/apiextensions/storageversion/cmd/migrate/config.go
index 2507e737d..5d3bb5560 100644
--- a/apiextensions/storageversion/cmd/migrate/config.go
+++ b/apiextensions/storageversion/cmd/migrate/config.go
@@ -31,7 +31,7 @@ import (
 
 func configOrDie() *rest.Config {
 	var (
-		masterURL = flag.String("master", "",
+		serverURL = flag.String("server", "",
 			"The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.")
 		kubeconfig = flag.String("kubeconfig", "",
 			"Path to a kubeconfig. Only required if out-of-cluster.")
@@ -39,7 +39,7 @@ func configOrDie() *rest.Config {
 
 	flag.Parse()
 
-	cfg, err := getConfig(*masterURL, *kubeconfig)
+	cfg, err := getConfig(*serverURL, *kubeconfig)
 	if err != nil {
 		panic(fmt.Sprintf("Error building kubeconfig: %v", err))
 	}
@@ -49,17 +49,17 @@ func configOrDie() *rest.Config {
 
 // getConfig returns a rest.Config to be used for kubernetes client creation.
 // It does so in the following order:
-//   1. Use the passed kubeconfig/masterURL.
+//   1. Use the passed kubeconfig/serverURL.
 //   2. Fallback to the KUBECONFIG environment variable.
 //   3. Fallback to in-cluster config.
 //   4. Fallback to the ~/.kube/config.
-func getConfig(masterURL, kubeconfig string) (*rest.Config, error) {
+func getConfig(serverURL, kubeconfig string) (*rest.Config, error) {
 	if kubeconfig == "" {
 		kubeconfig = os.Getenv("KUBECONFIG")
 	}
 	// If we have an explicit indication of where the kubernetes config lives, read that.
 	if kubeconfig != "" {
-		return clientcmd.BuildConfigFromFlags(masterURL, kubeconfig)
+		return clientcmd.BuildConfigFromFlags(serverURL, kubeconfig)
 	}
 	// If not, try the in-cluster config.
 	if c, err := rest.InClusterConfig(); err == nil {
diff --git a/injection/sharedmain/main.go b/injection/sharedmain/main.go
index af1baa774..e27bb8763 100644
--- a/injection/sharedmain/main.go
+++ b/injection/sharedmain/main.go
@@ -58,11 +58,11 @@ import (
 
 // GetConfig returns a rest.Config to be used for kubernetes client creation.
 // It does so in the following order:
-//   1. Use the passed kubeconfig/masterURL.
+//   1. Use the passed kubeconfig/serverURL.
 //   2. Fallback to the KUBECONFIG environment variable.
 //   3. Fallback to in-cluster config.
 //   4. Fallback to the ~/.kube/config.
-func GetConfig(masterURL, kubeconfig string) (*rest.Config, error) {
+func GetConfig(serverURL, kubeconfig string) (*rest.Config, error) {
 	if kubeconfig == "" {
 		kubeconfig = os.Getenv("KUBECONFIG")
 	}
@@ -75,7 +75,7 @@ func GetConfig(masterURL, kubeconfig string) (*rest.Config, error) {
 
 	// If we have an explicit indication of where the kubernetes config lives, read that.
 	if kubeconfig != "" {
-		c, err := clientcmd.BuildConfigFromFlags(masterURL, kubeconfig)
+		c, err := clientcmd.BuildConfigFromFlags(serverURL, kubeconfig)
 		if err != nil {
 			return nil, err
 		}
@@ -305,7 +305,7 @@ func flush(logger *zap.SugaredLogger) {
 // dies by calling log.Fatalf.
 func ParseAndGetConfigOrDie() *rest.Config {
 	var (
-		masterURL = flag.String("master", "",
+		serverURL = flag.String("server", "",
 			"The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.")
 		kubeconfig = flag.String("kubeconfig", "",
 			"Path to a kubeconfig. Only required if out-of-cluster.")
@@ -313,7 +313,7 @@ func ParseAndGetConfigOrDie() *rest.Config {
 	klog.InitFlags(flag.CommandLine)
 	flag.Parse()
 
-	cfg, err := GetConfig(*masterURL, *kubeconfig)
+	cfg, err := GetConfig(*serverURL, *kubeconfig)
 	if err != nil {
 		log.Fatalf("Error building kubeconfig: %v", err)
 	}
diff --git a/leaderelection/chaosduck/main.go b/leaderelection/chaosduck/main.go
index eeb20383f..5f21d4cb6 100644
--- a/leaderelection/chaosduck/main.go
+++ b/leaderelection/chaosduck/main.go
@@ -35,6 +35,7 @@ import (
 	kubeclient "knative.dev/pkg/client/injection/kube/client"
 	"knative.dev/pkg/injection/sharedmain"
 	"knative.dev/pkg/kflag"
+	"knative.dev/pkg/signals"
 	"knative.dev/pkg/system"
 )
 
@@ -115,7 +116,8 @@ func quack(ctx context.Context, kc kubernetes.Interface, component string, leade
 }
 
 func main() {
-	ctx := sharedmain.EnableInjectionOrDie(nil, nil)
+	ctx := signals.NewContext()
+	ctx = sharedmain.EnableInjectionOrDie(ctx, nil)
 
 	kc := kubeclient.Get(ctx)
 
diff --git a/metrics/config_observability.go b/metrics/config_observability.go
index 12df827de..177b35cb1 100644
--- a/metrics/config_observability.go
+++ b/metrics/config_observability.go
@@ -35,9 +35,13 @@ const (
 	defaultRequestMetricsBackend = "prometheus"
 
 	// The env var name for config-observability
-	configMapNameEnv  = "CONFIG_OBSERVABILITY_NAME"
-	reqLogTemplateKey = "logging.request-log-template"
-	enableReqLogKey   = "logging.enable-request-log"
+	configMapNameEnv = "CONFIG_OBSERVABILITY_NAME"
+
+	// ReqLogTemplateKey is the CM key for the request log template.
+	ReqLogTemplateKey = "logging.request-log-template"
+
+	// EnableReqLogKey is the CM key to enable request log.
+	EnableReqLogKey = "logging.enable-request-log"
 )
 
 // ObservabilityConfig contains the configuration defined in the observability ConfigMap.
@@ -88,8 +92,8 @@ func NewObservabilityConfigFromConfigMap(configMap *corev1.ConfigMap) (*Observab
 	if err := cm.Parse(configMap.Data,
 		cm.AsBool("logging.enable-var-log-collection", &oc.EnableVarLogCollection),
 		cm.AsString("logging.revision-url-template", &oc.LoggingURLTemplate),
-		cm.AsString(reqLogTemplateKey, &oc.RequestLogTemplate),
-		cm.AsBool(enableReqLogKey, &oc.EnableRequestLog),
+		cm.AsString(ReqLogTemplateKey, &oc.RequestLogTemplate),
+		cm.AsBool(EnableReqLogKey, &oc.EnableRequestLog),
 		cm.AsBool("logging.enable-probe-request-log", &oc.EnableProbeRequestLog),
 		cm.AsString("metrics.request-metrics-backend-destination", &oc.RequestMetricsBackend),
 		cm.AsBool("profiling.enable", &oc.EnableProfiling),
@@ -99,7 +103,7 @@ func NewObservabilityConfigFromConfigMap(configMap *corev1.ConfigMap) (*Observab
 	}
 
 	if oc.RequestLogTemplate == "" && oc.EnableRequestLog {
-		return nil, fmt.Errorf("%q was set to true, but no %q was specified", enableReqLogKey, reqLogTemplateKey)
+		return nil, fmt.Errorf("%q was set to true, but no %q was specified", EnableReqLogKey, ReqLogTemplateKey)
 	}
 
 	if oc.RequestLogTemplate != "" {
diff --git a/resolver/addressable_resolver.go b/resolver/addressable_resolver.go
index d15632f23..14c59a603 100644
--- a/resolver/addressable_resolver.go
+++ b/resolver/addressable_resolver.go
@@ -22,6 +22,7 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/types"
 
@@ -137,16 +138,17 @@ func (r *URIResolver) URIFromKReference(ref *duckv1.KReference, parent interface
 // URIFromObjectReference resolves an ObjectReference to a URI string.
 func (r *URIResolver) URIFromObjectReference(ref *corev1.ObjectReference, parent interface{}) (*apis.URL, error) {
 	if ref == nil {
-		return nil, errors.New("ref is nil")
+		return nil, apierrs.NewBadRequest("ref is nil")
 	}
 
+	gvr, _ := meta.UnsafeGuessKindToResource(ref.GroupVersionKind())
 	if err := r.tracker.TrackReference(tracker.Reference{
 		APIVersion: ref.APIVersion,
 		Kind:       ref.Kind,
 		Namespace:  ref.Namespace,
 		Name:       ref.Name,
 	}, parent); err != nil {
-		return nil, fmt.Errorf("failed to track %+v: %w", ref, err)
+		return nil, apierrs.NewNotFound(gvr.GroupResource(), ref.Name)
 	}
 
 	// K8s Services are special cased. They can be called, even though they do not satisfy the
@@ -161,30 +163,29 @@ func (r *URIResolver) URIFromObjectReference(ref *corev1.ObjectReference, parent
 		return url, nil
 	}
 
-	gvr, _ := meta.UnsafeGuessKindToResource(ref.GroupVersionKind())
 	_, lister, err := r.informerFactory.Get(gvr)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get lister for %+v: %w", gvr, err)
+		return nil, apierrs.NewNotFound(gvr.GroupResource(), "Lister")
 	}
 
 	obj, err := lister.ByNamespace(ref.Namespace).Get(ref.Name)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get ref %+v: %w", ref, err)
+		return nil, apierrs.NewNotFound(gvr.GroupResource(), ref.Name)
 	}
 
 	addressable, ok := obj.(*duckv1.AddressableType)
 	if !ok {
-		return nil, fmt.Errorf("%+v (%T) is not an AddressableType", ref, ref)
+		return nil, apierrs.NewBadRequest(fmt.Sprintf("%+v (%T) is not an AddressableType", ref, ref))
 	}
 	if addressable.Status.Address == nil {
-		return nil, fmt.Errorf("address not set for %+v", ref)
+		return nil, apierrs.NewBadRequest(fmt.Sprintf("address not set for %+v", ref))
 	}
 	url := addressable.Status.Address.URL
 	if url == nil {
-		return nil, fmt.Errorf("URL missing in address of %+v", ref)
+		return nil, apierrs.NewBadRequest(fmt.Sprintf("URL missing in address of %+v", ref))
 	}
 	if url.Host == "" {
-		return nil, fmt.Errorf("hostname missing in address of %+v", ref)
+		return nil, apierrs.NewBadRequest(fmt.Sprintf("hostname missing in address of %+v", ref))
 	}
 	return url, nil
 }


Test patch:
diff --git a/metrics/config_observability_test.go b/metrics/config_observability_test.go
index 9e752bd5a..075db67b9 100644
--- a/metrics/config_observability_test.go
+++ b/metrics/config_observability_test.go
@@ -46,9 +46,9 @@ func TestObservabilityConfiguration(t *testing.T) {
 		data: map[string]string{
 			"logging.enable-probe-request-log":            "true",
 			"logging.enable-var-log-collection":           "true",
-			"logging.request-log-template":                `{"requestMethod": "{{.Request.Method}}"}`,
+			ReqLogTemplateKey:                             `{"requestMethod": "{{.Request.Method}}"}`,
 			"logging.revision-url-template":               "https://logging.io",
-			"logging.enable-request-log":                  "true",
+			EnableReqLogKey:                               "true",
 			"metrics.request-metrics-backend-destination": "stackdriver",
 			"profiling.enable":                            "true",
 		},
@@ -59,13 +59,13 @@ func TestObservabilityConfiguration(t *testing.T) {
 		name:    "invalid request log template",
 		wantErr: true,
 		data: map[string]string{
-			"logging.request-log-template": `{{ something }}`,
+			ReqLogTemplateKey: `{{ something }}`,
 		},
 	}, {
 		name: "observability configuration with request log set and template default",
 		data: map[string]string{
 			"logging.enable-probe-request-log":            "true",
-			"logging.enable-request-log":                  "true",
+			EnableReqLogKey:                               "true",
 			"logging.enable-var-log-collection":           "true",
 			"logging.revision-url-template":               "https://logging.io",
 			"metrics.request-metrics-backend-destination": "stackdriver",
@@ -91,9 +91,9 @@ func TestObservabilityConfiguration(t *testing.T) {
 		},
 		data: map[string]string{
 			"logging.enable-probe-request-log":            "true",
-			"logging.enable-request-log":                  "false",
+			EnableReqLogKey:                               "false",
 			"logging.enable-var-log-collection":           "true",
-			"logging.request-log-template":                "",
+			ReqLogTemplateKey:                             "",
 			"logging.revision-url-template":               "https://logging.io",
 			"metrics.request-metrics-backend-destination": "stackdriver",
 			"profiling.enable":                            "true",
@@ -103,16 +103,15 @@ func TestObservabilityConfiguration(t *testing.T) {
 		wantErr: true,
 		data: map[string]string{
 			"logging.enable-probe-request-log":            "true",
-			"logging.enable-request-log":                  "true",
+			EnableReqLogKey:                               "true",
 			"logging.enable-var-log-collection":           "true",
-			"logging.request-log-template":                "",
+			ReqLogTemplateKey:                             "",
 			"logging.revision-url-template":               "https://logging.io",
 			"metrics.request-metrics-backend-destination": "stackdriver",
 			"profiling.enable":                            "true",
 		},
 	}, {
-		name:    "observability configuration with request log not set and with template set",
-		wantErr: false,
+		name: "observability configuration with request log not set and with template set",
 		wantConfig: &ObservabilityConfig{
 			EnableProbeRequestLog:  true,
 			EnableProfiling:        true,
@@ -124,7 +123,7 @@ func TestObservabilityConfiguration(t *testing.T) {
 		data: map[string]string{
 			"logging.enable-probe-request-log":            "true",
 			"logging.enable-var-log-collection":           "true",
-			"logging.request-log-template":                `{"requestMethod": "{{.Request.Method}}"}`,
+			ReqLogTemplateKey:                             `{"requestMethod": "{{.Request.Method}}"}`,
 			"logging.revision-url-template":               "https://logging.io",
 			"metrics.request-metrics-backend-destination": "stackdriver",
 			"profiling.enable":                            "true",
diff --git a/metrics/exporter_test.go b/metrics/exporter_test.go
index 7cef82d6d..d84863125 100644
--- a/metrics/exporter_test.go
+++ b/metrics/exporter_test.go
@@ -214,7 +214,14 @@ func TestInterlevedExporters(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	expectNoPromSrv(t)
+
+	// Expect no prometheus server
+	time.Sleep(200 * time.Millisecond)
+	srv := getCurPromSrv()
+	if srv != nil {
+		t.Error("expected no server for stackdriver exporter")
+	}
+
 	// Then switch to prometheus exporter
 	_, _, err = newMetricsExporter(&metricsConfig{
 		domain:             servingDomain,
diff --git a/metrics/prometheus_exporter_test.go b/metrics/prometheus_exporter_test.go
index 5a7785a2f..6a416fb3a 100644
--- a/metrics/prometheus_exporter_test.go
+++ b/metrics/prometheus_exporter_test.go
@@ -67,11 +67,3 @@ func expectPromSrv(t *testing.T, expectedAddr string) {
 		t.Errorf("metrics port addresses diff, got=%v, want=%v", got, want)
 	}
 }
-
-func expectNoPromSrv(t *testing.T) {
-	time.Sleep(200 * time.Millisecond)
-	srv := getCurPromSrv()
-	if srv != nil {
-		t.Error("expected no server for stackdriver exporter")
-	}
-}
diff --git a/resolver/addressable_resolver_test.go b/resolver/addressable_resolver_test.go
index a989da260..b7d94f069 100644
--- a/resolver/addressable_resolver_test.go
+++ b/resolver/addressable_resolver_test.go
@@ -328,7 +328,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantErr: fmt.Sprintf("address not set for %+v", getUnaddressableRef()),
 		}, "notFound": {
 			dest:    duckv1beta1.Destination{Ref: getUnaddressableRef()},
-			wantErr: fmt.Sprintf("failed to get ref %+v: %s %q not found", getUnaddressableRef(), unaddressableResource, unaddressableName),
+			wantErr: fmt.Sprintf("%s %q not found", unaddressableResource, unaddressableName),
 		}}
 
 	for n, tc := range tests {
@@ -510,7 +510,7 @@ func TestGetURIDestinationV1(t *testing.T) {
 			wantErr: fmt.Sprintf("address not set for %+v", getUnaddressableRef()),
 		}, "notFound": {
 			dest:    duckv1.Destination{Ref: getUnaddressableKnativeRef()},
-			wantErr: fmt.Sprintf("failed to get ref %+v: %s %q not found", getUnaddressableRef(), unaddressableResource, unaddressableName),
+			wantErr: fmt.Sprintf("%s %q not found", unaddressableResource, unaddressableName),
 		}}
 
 	for n, tc := range tests {
@@ -548,12 +548,11 @@ func TestURIFromObjectReferenceErrors(t *testing.T) {
 	}{"nil": {
 		wantErr: "ref is nil",
 	}, "fail tracker with bad object": {
-		ref: getInvalidObjectReference(),
-		wantErr: fmt.Sprintf(`failed to track %+v: invalid Reference:
-Namespace: a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name',  or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')`, getInvalidObjectReference()),
+		ref:     getInvalidObjectReference(),
+		wantErr: `sinks.duck.knative.dev "testsink" not found`,
 	}, "fail get": {
 		ref:     getAddressableRef(),
-		wantErr: fmt.Sprintf(`failed to get ref %+v: sinks.duck.knative.dev "testsink" not found`, getAddressableRef()),
+		wantErr: `sinks.duck.knative.dev "testsink" not found`,
 	}}
 
 	for n, tc := range tests {
diff --git a/test/ghutil/fakeghutil/fakeghutil.go b/test/ghutil/fakeghutil/fakeghutil.go
index 3f8243155..2829b1cef 100644
--- a/test/ghutil/fakeghutil/fakeghutil.go
+++ b/test/ghutil/fakeghutil/fakeghutil.go
@@ -241,11 +241,11 @@ func (fgc *FakeGithubClient) ListCommits(org, repo string, ID int) ([]*github.Re
 
 // ListFiles lists files from a pull request
 func (fgc *FakeGithubClient) ListFiles(org, repo string, ID int) ([]*github.CommitFile, error) {
-	var res []*github.CommitFile
 	commits, err := fgc.ListCommits(org, repo, ID)
 	if nil != err {
 		return nil, err
 	}
+	res := make([]*github.CommitFile, 0, len(commits))
 	for _, commit := range commits {
 		files, ok := fgc.CommitFiles[*commit.SHA]
 		if !ok {
diff --git a/test/logging/example_tlogger_test.go b/test/logging/example_tlogger_test.go
index 88d4f5c39..6c194b1e4 100644
--- a/test/logging/example_tlogger_test.go
+++ b/test/logging/example_tlogger_test.go
@@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+// nolint
 package logging_test
 
 import (
@@ -34,7 +35,7 @@ var (
 
 func init() { someStruct = testStruct{"hello", 42.0} }
 
-// nolint:govet // godoc limitation; this would be named Test*(), of course
+// godoc limitation; this would be named Test*(), of course
 func SampleTest(legacy *testing.T) {
 	// Get our TLogger and ready the cleanup function
 	t, cancel := logging.NewTLogger(legacy)
diff --git a/testutils/clustermanager/perf-tests/pkg/benchmark.go b/testutils/clustermanager/perf-tests/pkg/benchmark.go
index 221e18584..ea8162359 100644
--- a/testutils/clustermanager/perf-tests/pkg/benchmark.go
+++ b/testutils/clustermanager/perf-tests/pkg/benchmark.go
@@ -40,7 +40,7 @@ const (
 )
 
 // backupLocations are used in retrying cluster creation, if stockout happens in one location.
-// TODO(chizhg): it's currently not used, use it in the cluster creation retry logic.
+// nolint // TODO(chizhg): it's currently not used, use it in the cluster creation retry logic.
 var backupLocations = []string{"us-west1", "us-west2", "us-east1"}
 
 // GKECluster saves the config information for the GKE cluster
diff --git a/testutils/clustermanager/perf-tests/pkg/cluster.go b/testutils/clustermanager/perf-tests/pkg/cluster.go
index 90f0ccfc5..473f072ba 100644
--- a/testutils/clustermanager/perf-tests/pkg/cluster.go
+++ b/testutils/clustermanager/perf-tests/pkg/cluster.go
@@ -38,7 +38,6 @@ const (
 	// known cluster status
 	// TODO(chizhg): move these status constants to gke package
 	statusProvisioning = "PROVISIONING"
-	statusRunning      = "RUNNING"
 	statusStopping     = "STOPPING"
 )