Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply various fixes pointed out by staticcheck. #4242

Merged
merged 12 commits into from
Jun 4, 2019
2 changes: 1 addition & 1 deletion cmd/activator/request_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 1 addition & 2 deletions cmd/networking/certmanager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ import (
)

const (
threadsPerController = 2
component = "controller-certificate-cert-manager"
component = "controller-certificate-cert-manager"
)

var (
Expand Down
3 changes: 1 addition & 2 deletions cmd/networking/istio/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ import (
)

const (
threadsPerController = 2
component = "controller-clusteringress-istio"
component = "controller-clusteringress-istio"
)

var (
Expand Down
6 changes: 0 additions & 6 deletions pkg/activator/handler/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 0 additions & 10 deletions pkg/apis/serving/v1alpha1/revision_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -62,8 +61,6 @@ var revCondSet = apis.NewLivingConditionSet(
RevisionConditionContainerHealthy,
)

var buildCondSet = duckv1alpha1.NewBatchConditionSet()

func (r *Revision) GetGroupVersionKind() schema.GroupVersionKind {
return SchemeGroupVersion.WithKind("Revision")
}
Expand Down Expand Up @@ -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())
}
Expand Down
3 changes: 0 additions & 3 deletions pkg/apis/serving/v1beta1/configuration_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 0 additions & 3 deletions pkg/apis/serving/v1beta1/revision_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 0 additions & 3 deletions pkg/apis/serving/v1beta1/route_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 0 additions & 3 deletions pkg/apis/serving/v1beta1/service_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 4 additions & 4 deletions pkg/autoscaler/autoscaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ package autoscaler
import (
"errors"
"fmt"
"github.com/knative/serving/pkg/resources"
"testing"
"time"

"github.com/knative/serving/pkg/resources"

. "github.com/knative/pkg/logging/testing"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -31,9 +32,8 @@ import (
)

const (
stableWindow = 60 * time.Second
panicWindow = 6 * time.Second
activatorPodName = "activator"
stableWindow = 60 * time.Second
panicWindow = 6 * time.Second
)

var (
Expand Down
12 changes: 0 additions & 12 deletions pkg/autoscaler/multiscaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
1 change: 0 additions & 1 deletion pkg/autoscaler/stats_scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ var cacheDisabledClient = &http.Client{
// for details.
type ServiceScraper struct {
sClient scrapeClient
sampleSizeFunc SampleSizeFunc
counter resources.ReadyPodCounter
url string
namespace string
Expand Down
2 changes: 1 addition & 1 deletion pkg/deployment/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions pkg/http/request_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
13 changes: 8 additions & 5 deletions pkg/reconciler/autoscaling/kpa/kpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops :)

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
}
Expand Down Expand Up @@ -343,8 +347,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")
}
}
Expand Down
14 changes: 0 additions & 14 deletions pkg/reconciler/autoscaling/kpa/kpa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/autoscaling/kpa/scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
18 changes: 0 additions & 18 deletions pkg/reconciler/autoscaling/kpa/scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/certificate/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 0 additions & 23 deletions pkg/reconciler/clusteringress/clusteringress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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{
Expand Down
Loading