Apply various fixes pointed out by staticcheck. (knative#4242)

* Transform string(buf.Bytes()) to buf.String(). * Remove a bunch of unused code. * Fix error capitalization. * Fix issue with error overlapping. * Fix deprecated usage of Apps without version. * Fix file permission resolution. * Fix comparison to boolean. * Fix issue with variable never being used. * Remove unused conditionsets. * Fix error checks after fixing capitalization. * Remove unused values in performance tests. * Remove some more unused code.
joshrider · Jun 5, 2019 · 4f8ba04 · 4f8ba04
1 parent 5a6de23
commit 4f8ba04
Show file tree

Hide file tree

Showing 39 changed files with 44 additions and 204 deletions.
diff --git a/cmd/activator/request_log_test.go b/cmd/activator/request_log_test.go
@@ -105,7 +105,7 @@ func TestUpdateRequestLogFromConfigMap(t *testing.T) {
 			}
 			handler.ServeHTTP(resp, req)
 
-			got := string(buf.Bytes())
+			got := buf.String()
 			if got != test.want {
 				t.Errorf("got '%v', want '%v'", got, test.want)
 			}

diff --git a/cmd/networking/certmanager/main.go b/cmd/networking/certmanager/main.go
@@ -36,8 +36,7 @@ import (
 )
 
 const (
-	threadsPerController = 2
-	component            = "controller-certificate-cert-manager"
+	component = "controller-certificate-cert-manager"
 )
 
 var (

diff --git a/cmd/networking/istio/main.go b/cmd/networking/istio/main.go
@@ -35,8 +35,7 @@ import (
 )
 
 const (
-	threadsPerController = 2
-	component            = "controller-clusteringress-istio"
+	component = "controller-clusteringress-istio"
 )
 
 var (

diff --git a/pkg/activator/handler/handler_test.go b/pkg/activator/handler/handler_test.go
@@ -567,12 +567,6 @@ func sendRequests(count int, namespace, revName string, respCh chan *httptest.Re
 	}
 }
 
-// getHandler returns an already setup activationHandler. The roundtripper is controlled
-// via the given `lockerCh`.
-func getHandler(throttler *activator.Throttler, lockerCh chan struct{}, t *testing.T) activationHandler {
-	return activationHandler{}
-}
-
 // getRT returns a roundTripper. probeErr if set is an error response returned to probe requests. probeCode is the
 // response code of probe requests. probeResp if set is the body to respond with for probe requests.
 // err if set is an error response returned for forwarded requests. body is the response body of forwarded requests.

diff --git a/pkg/apis/serving/v1alpha1/revision_lifecycle.go b/pkg/apis/serving/v1alpha1/revision_lifecycle.go
@@ -22,7 +22,6 @@ import (
 	"time"
 
 	"github.com/knative/pkg/apis"
-	duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
 	duckv1beta1 "github.com/knative/pkg/apis/duck/v1beta1"
 	net "github.com/knative/serving/pkg/apis/networking"
 	"github.com/knative/serving/pkg/apis/serving"
@@ -62,8 +61,6 @@ var revCondSet = apis.NewLivingConditionSet(
 	RevisionConditionContainerHealthy,
 )
 
-var buildCondSet = duckv1alpha1.NewBatchConditionSet()
-
 func (r *Revision) GetGroupVersionKind() schema.GroupVersionKind {
 	return SchemeGroupVersion.WithKind("Revision")
 }
@@ -236,13 +233,6 @@ func (e LastPinnedParseError) Error() string {
 	return fmt.Sprintf("%v lastPinned value: %q", e.Type, e.Value)
 }
 
-// +k8s:deepcopy-gen=false
-type configurationGenerationParseError AnnotationParseError
-
-func (e configurationGenerationParseError) Error() string {
-	return fmt.Sprintf("%v configurationGeneration value: %q", e.Type, e.Value)
-}
-
 func RevisionLastPinnedString(t time.Time) string {
 	return fmt.Sprintf("%d", t.Unix())
 }

diff --git a/pkg/apis/serving/v1beta1/configuration_lifecycle.go b/pkg/apis/serving/v1beta1/configuration_lifecycle.go
@@ -17,12 +17,9 @@ limitations under the License.
 package v1beta1
 
 import (
-	"github.com/knative/pkg/apis"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
-var confCondSet = apis.NewLivingConditionSet()
-
 // GetGroupVersionKind returns the GroupVersionKind.
 func (r *Configuration) GetGroupVersionKind() schema.GroupVersionKind {
 	return SchemeGroupVersion.WithKind("Configuration")

diff --git a/pkg/apis/serving/v1beta1/revision_lifecycle.go b/pkg/apis/serving/v1beta1/revision_lifecycle.go
@@ -17,12 +17,9 @@ limitations under the License.
 package v1beta1
 
 import (
-	"github.com/knative/pkg/apis"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
-var revCondSet = apis.NewLivingConditionSet()
-
 // GetGroupVersionKind returns the GroupVersionKind.
 func (r *Revision) GetGroupVersionKind() schema.GroupVersionKind {
 	return SchemeGroupVersion.WithKind("Revision")

diff --git a/pkg/apis/serving/v1beta1/route_lifecycle.go b/pkg/apis/serving/v1beta1/route_lifecycle.go
@@ -17,12 +17,9 @@ limitations under the License.
 package v1beta1
 
 import (
-	"github.com/knative/pkg/apis"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
-var routeCondSet = apis.NewLivingConditionSet()
-
 // GetGroupVersionKind returns the GroupVersionKind.
 func (r *Route) GetGroupVersionKind() schema.GroupVersionKind {
 	return SchemeGroupVersion.WithKind("Route")

diff --git a/pkg/apis/serving/v1beta1/service_lifecycle.go b/pkg/apis/serving/v1beta1/service_lifecycle.go
@@ -17,12 +17,9 @@ limitations under the License.
 package v1beta1
 
 import (
-	"github.com/knative/pkg/apis"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
-var serviceCondSet = apis.NewLivingConditionSet()
-
 // GetGroupVersionKind returns the GroupVersionKind.
 func (s *Service) GetGroupVersionKind() schema.GroupVersionKind {
 	return SchemeGroupVersion.WithKind("Service")

diff --git a/pkg/autoscaler/autoscaler_test.go b/pkg/autoscaler/autoscaler_test.go
@@ -32,9 +32,8 @@ import (
 )
 
 const (
-	stableWindow     = 60 * time.Second
-	panicWindow      = 6 * time.Second
-	activatorPodName = "activator"
+	stableWindow = 60 * time.Second
+	panicWindow  = 6 * time.Second
 )
 
 var (

diff --git a/pkg/autoscaler/multiscaler_test.go b/pkg/autoscaler/multiscaler_test.go
@@ -36,10 +36,6 @@ const (
 	tickTimeout  = 50 * time.Millisecond
 )
 
-var testStatMessage = StatMessage{
-	Key: testKPAKey,
-}
-
 // watchFunc generates a function to assert the changes happening in the multiscaler.
 func watchFunc(ctx context.Context, ms *MultiScaler, decider *Decider, desiredScale int, errCh chan error) func(key string) {
 	metricKey := fmt.Sprintf("%s/%s", decider.Namespace, decider.Name)
@@ -386,14 +382,6 @@ func (u *fakeUniScaler) Record(ctx context.Context, stat Stat) {
 	u.lastStat = stat
 }
 
-func (u *fakeUniScaler) checkLastStat(t *testing.T, stat Stat) {
-	t.Helper()
-
-	if u.lastStat != stat {
-		t.Fatalf("Last statistic recorded was %#v instead of expected statistic %#v", u.lastStat, stat)
-	}
-}
-
 func (u *fakeUniScaler) Update(DeciderSpec) error {
 	return nil
 }

diff --git a/pkg/autoscaler/stats_scraper.go b/pkg/autoscaler/stats_scraper.go
@@ -73,7 +73,6 @@ var cacheDisabledClient = &http.Client{
 // for details.
 type ServiceScraper struct {
 	sClient             scrapeClient
-	sampleSizeFunc      SampleSizeFunc
 	counter             resources.ReadyPodCounter
 	url                 string
 	namespace           string

diff --git a/pkg/deployment/config.go b/pkg/deployment/config.go
@@ -38,7 +38,7 @@ func NewConfigFromMap(configMap map[string]string) (*Config, error) {
 	nc := &Config{}
 	qsideCarImage, ok := configMap[QueueSidecarImageKey]
 	if !ok {
-		return nil, errors.New("Queue sidecar image is missing")
+		return nil, errors.New("queue sidecar image is missing")
 	}
 	nc.QueueSidecarImage = qsideCarImage
 

diff --git a/pkg/http/request_log_test.go b/pkg/http/request_log_test.go
@@ -96,7 +96,7 @@ func TestRequestLogHandler(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, test.url, bytes.NewBufferString(test.body))
 				handler.ServeHTTP(resp, req)
 
-				got := string(buf.Bytes())
+				got := buf.String()
 				if got != test.want {
 					t.Errorf("got '%v', want '%v'", got, test.want)
 				}
@@ -124,7 +124,7 @@ func TestPanickingHandler(t *testing.T) {
 			t.Error("want ServeHTTP to panic, got nothing.")
 		}
 
-		got := string(buf.Bytes())
+		got := buf.String()
 		if want := "http://example.com\n"; got != want {
 			t.Errorf("got '%v', want '%v'", got, want)
 		}
@@ -147,7 +147,7 @@ func TestFailedTemplateExecution(t *testing.T) {
 	req := httptest.NewRequest(http.MethodGet, "http://example.com", bytes.NewBufferString("test"))
 	handler.ServeHTTP(resp, req)
 
-	got := string(buf.Bytes())
+	got := buf.String()
 	if want := "Invalid request log template: "; !strings.HasPrefix(got, want) {
 		t.Errorf("got: '%v', want: '%v'", got, want)
 	}
@@ -215,7 +215,7 @@ func TestSetTemplate(t *testing.T) {
 				resp := httptest.NewRecorder()
 				req := httptest.NewRequest(http.MethodPost, url, bytes.NewBufferString(body))
 				handler.ServeHTTP(resp, req)
-				got := string(buf.Bytes())
+				got := buf.String()
 				if got != test.want {
 					t.Errorf("got '%v', want '%v'", got, test.want)
 				}

diff --git a/pkg/reconciler/autoscaling/kpa/kpa.go b/pkg/reconciler/autoscaling/kpa/kpa.go
@@ -81,9 +81,13 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 	original, err := c.paLister.PodAutoscalers(namespace).Get(name)
 	if errors.IsNotFound(err) {
 		logger.Debug("PA no longer exists")
-		err = c.kpaDeciders.Delete(ctx, namespace, name)
-		err = c.metrics.Delete(ctx, namespace, name)
-		return err
+		if err := c.kpaDeciders.Delete(ctx, namespace, name); err != nil {
+			return err
+		}
+		if err := c.metrics.Delete(ctx, namespace, name); err != nil {
+			return err
+		}
+		return nil
 	} else if err != nil {
 		return err
 	}
@@ -310,8 +314,7 @@ func (c *Reconciler) reconcileMetric(ctx context.Context, pa *pav1alpha1.PodAuto
 	// Ignore status when reconciling
 	desiredMetric.Status = metric.Status
 	if !equality.Semantic.DeepEqual(desiredMetric, metric) {
-		metric, err = c.metrics.Update(ctx, desiredMetric)
-		if err != nil {
+		if _, err = c.metrics.Update(ctx, desiredMetric); err != nil {
 			return perrors.Wrap(err, "error updating metric")
 		}
 	}

diff --git a/pkg/reconciler/autoscaling/kpa/kpa_test.go b/pkg/reconciler/autoscaling/kpa/kpa_test.go
@@ -32,7 +32,6 @@ import (
 
 	"github.com/knative/pkg/configmap"
 	"github.com/knative/pkg/controller"
-	"github.com/knative/pkg/kmeta"
 	logtesting "github.com/knative/pkg/logging/testing"
 	"github.com/knative/pkg/ptr"
 	"github.com/knative/pkg/system"
@@ -42,7 +41,6 @@ import (
 	nv1a1 "github.com/knative/serving/pkg/apis/networking/v1alpha1"
 	"github.com/knative/serving/pkg/apis/serving"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
-	"github.com/knative/serving/pkg/apis/serving/v1beta1"
 	"github.com/knative/serving/pkg/autoscaler"
 	fakeKna "github.com/knative/serving/pkg/client/clientset/versioned/fake"
 	informers "github.com/knative/serving/pkg/client/informers/externalversions"
@@ -142,12 +140,6 @@ func markActive(pa *asv1a1.PodAutoscaler) {
 	pa.Status.MarkActive()
 }
 
-func sksWithOwnership(pa *asv1a1.PodAutoscaler) SKSOption {
-	return func(sks *nv1a1.ServerlessService) {
-		sks.ObjectMeta.OwnerReferences = []metav1.OwnerReference{*kmeta.NewControllerRef(pa)}
-	}
-}
-
 func kpa(ns, n string, opts ...PodAutoscalerOption) *asv1a1.PodAutoscaler {
 	rev := newTestRevision(ns, n)
 	kpa := revisionresources.MakeKPA(rev)
@@ -159,12 +151,6 @@ func kpa(ns, n string, opts ...PodAutoscalerOption) *asv1a1.PodAutoscaler {
 	return kpa
 }
 
-func withConcurrency(n int) PodAutoscalerOption {
-	return func(pa *asv1a1.PodAutoscaler) {
-		pa.Spec.ContainerConcurrency = v1beta1.RevisionContainerConcurrencyType(n)
-	}
-}
-
 func markResourceNotOwned(rType, name string) PodAutoscalerOption {
 	return func(pa *asv1a1.PodAutoscaler) {
 		pa.Status.MarkResourceNotOwned(rType, name)

diff --git a/pkg/reconciler/autoscaling/kpa/scaler.go b/pkg/reconciler/autoscaling/kpa/scaler.go
@@ -172,7 +172,7 @@ func (ks *scaler) handleScaleToZero(pa *pav1alpha1.PodAutoscaler, desiredScale i
 	//   b) The PA has been active for at least the stable window, after which it gets marked inactive
 	//   c) The PA has been inactive for at least the grace period
 
-	if config.EnableScaleToZero == false {
+	if !config.EnableScaleToZero {
 		return 1, true
 	}
 

diff --git a/pkg/reconciler/autoscaling/kpa/scaler_test.go b/pkg/reconciler/autoscaling/kpa/scaler_test.go
@@ -491,13 +491,6 @@ func kpaMarkInactive(pa *pav1alpha1.PodAutoscaler, ltt time.Time) {
 	pa.Status.Conditions[0].LastTransitionTime = apis.VolatileTime{Inner: metav1.NewTime(ltt)}
 }
 
-func kpaMarkActivating(pa *pav1alpha1.PodAutoscaler, ltt time.Time) {
-	pa.Status.MarkActivating("", "")
-
-	// This works because the conditions are sorted alphabetically
-	pa.Status.Conditions[0].LastTransitionTime = apis.VolatileTime{Inner: metav1.NewTime(ltt)}
-}
-
 func checkReplicas(t *testing.T, dynamicClient *fakedynamic.FakeDynamicClient, deployment *v1.Deployment, expectedScale int32) {
 	t.Helper()
 
@@ -522,17 +515,6 @@ func checkReplicas(t *testing.T, dynamicClient *fakedynamic.FakeDynamicClient, d
 	}
 }
 
-func checkNoScaling(t *testing.T, dynamicClient *fakedynamic.FakeDynamicClient) {
-	t.Helper()
-
-	for _, action := range dynamicClient.Actions() {
-		switch action.GetVerb() {
-		case "update":
-			t.Errorf("Unexpected update: %v", action)
-		}
-	}
-}
-
 func TestActivatorProbe(t *testing.T) {
 	oldRT := network.AutoTransport
 	defer func() {

diff --git a/pkg/reconciler/certificate/certificate.go b/pkg/reconciler/certificate/certificate.go
@@ -151,7 +151,7 @@ func (c *Reconciler) reconcileCMCertificate(ctx context.Context, knCert *v1alpha
 		return nil, err
 	} else if !metav1.IsControlledBy(desired, knCert) {
 		knCert.Status.MarkResourceNotOwned("CertManagerCertificate", desired.Name)
-		return nil, fmt.Errorf("Knative Certificate %s in namespace %s does not own CertManager Certificate: %s", knCert.Name, knCert.Namespace, desired.Name)
+		return nil, fmt.Errorf("knative Certificate %s in namespace %s does not own CertManager Certificate: %s", knCert.Name, knCert.Namespace, desired.Name)
 	} else if !equality.Semantic.DeepEqual(cmCert.Spec, desired.Spec) {
 		copy := cmCert.DeepCopy()
 		copy.Spec = desired.Spec

diff --git a/pkg/reconciler/clusteringress/clusteringress_test.go b/pkg/reconciler/clusteringress/clusteringress_test.go
@@ -54,7 +54,6 @@ import (
 	"github.com/knative/serving/pkg/reconciler"
 	"github.com/knative/serving/pkg/reconciler/clusteringress/config"
 	"github.com/knative/serving/pkg/reconciler/clusteringress/resources"
-	presources "github.com/knative/serving/pkg/resources"
 
 	. "github.com/knative/pkg/reconciler/testing"
 	. "github.com/knative/serving/pkg/reconciler/testing"
@@ -536,11 +535,6 @@ func patchAddFinalizerAction(ingressName, finalizer string) clientgotesting.Patc
 	return action
 }
 
-func addAnnotations(ing *v1alpha1.ClusterIngress, annos map[string]string) *v1alpha1.ClusterIngress {
-	ing.ObjectMeta.Annotations = presources.UnionMaps(annos, ing.ObjectMeta.Annotations)
-	return ing
-}
-
 type testConfigStore struct {
 	config *config.Config
 }
@@ -553,23 +547,6 @@ func (t *testConfigStore) WatchConfigs(w configmap.Watcher) {}
 
 var _ configStore = (*testConfigStore)(nil)
 
-func ReconcilerTestConfig() *config.Config {
-	return &config.Config{
-		Istio: &config.Istio{
-			IngressGateways: []config.Gateway{{
-				GatewayName: "knative-test-gateway",
-				ServiceURL:  network.GetServiceHostname("test-ingressgateway", "istio-system"),
-			}, {
-				GatewayName: "knative-ingress-gateway",
-				ServiceURL:  network.GetServiceHostname("istio-ingressgateway", "istio-system"),
-			}},
-		},
-		Network: &network.Config{
-			AutoTLS: false,
-		},
-	}
-}
-
 func ingressWithStatus(name string, generation int64, status v1alpha1.IngressStatus) *v1alpha1.ClusterIngress {
 	return &v1alpha1.ClusterIngress{
 		ObjectMeta: metav1.ObjectMeta{