Merge pull request #261 from lbbniu/feature/lbbniu/reviewdog

Add code static check with reviewdog

.github/workflows/go.yml

@@ -184,6 +184,3 @@ jobs:
         with:
           # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
           version: v1.45.2
-
-          # Optional: golangci-lint command line arguments.
-          args: --timeout 30m --disable-all -E deadcode -E unused -E varcheck -E ineffassign -E goimports -E gofmt -E misspell -E unparam -E unconvert -E govet -E errcheck -E structcheck

.golangci.yaml

@@ -0,0 +1,29 @@
+run:
+  timeout: 30m
+
+linters:
+  disable-all: true
+  enable:
+    - deadcode
+    - unused
+    - varcheck
+    - ineffassign
+    - goimports
+    - gofmt
+    - misspell
+    - unparam
+    - unconvert
+    - govet
+    - errcheck
+    - structcheck
+    - staticcheck
+
+linters-settings:
+  staticcheck:
+    go: "1.17"
+    checks: [
+      "all",
+      "-SA1019", # TODO(fix) Using a deprecated function, variable, constant or field
+    ]
+  unused:
+    go: "1.17"

Makefile

@@ -87,21 +87,7 @@ tidy:
 
 .PHONY: lint
 lint: golangci-lint  ## Run golang lint against code
-	@$(GOLANG_LINT) run \
-      --timeout 30m \
-      --disable-all \
-      -E deadcode \
-      -E unused \
-      -E varcheck \
-      -E ineffassign \
-      -E goimports \
-      -E gofmt \
-      -E misspell \
-      -E unparam \
-      -E unconvert \
-      -E govet \
-      -E errcheck \
-      -E structcheck
+	@$(GOLANG_LINT) run ./...
 
 .PHONY: test
 test: fmt vet lint ## Run tests.

pkg/autoscaling/estimator/oom.go

@@ -34,7 +34,7 @@ func (e *OOMResourceEstimator) GetResourceEstimation(evpa *autoscalingapi.Effect
 	}
 
 	// ignore too old oom events
-	if oomRecord != nil && time.Now().Sub(oomRecord.OOMAt) <= (time.Hour*24*7) {
+	if oomRecord != nil && time.Since(oomRecord.OOMAt) <= (time.Hour*24*7) {
 		memoryOOM := oomRecord.Memory.Value()
 		recommendResource := corev1.ResourceList{}
 		var memoryNeeded recommendermodel.ResourceAmount
@@ -47,7 +47,7 @@ func (e *OOMResourceEstimator) GetResourceEstimation(evpa *autoscalingapi.Effect
 		} else {
 			oomBumpUpRatio, err := strconv.ParseFloat(bumpUpRatio, 64)
 			if err != nil {
-				return nil, fmt.Errorf("Parse bumpUpRatio failed: %v. ", err)
+				return nil, fmt.Errorf("parse bumpUpRatio failed: %v. ", err)
 			}
 			memoryNeeded = recommendermodel.ScaleResource(recommendermodel.ResourceAmount(memoryOOM), oomBumpUpRatio)
 		}

pkg/controller/analytics/analytics_controller.go

@@ -7,7 +7,6 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -111,7 +110,7 @@ func (c *Controller) DoAnalytics(ctx context.Context, analytics *analysisv1alph1
 
 	identities, err := c.GetIdentities(ctx, analytics)
 	if err != nil {
-		c.Recorder.Event(analytics, v1.EventTypeNormal, "FailedSelectResource", err.Error())
+		c.Recorder.Event(analytics, corev1.EventTypeNormal, "FailedSelectResource", err.Error())
 		msg := fmt.Sprintf("Failed to get idenitities, Analytics %s error %v", klog.KObj(analytics), err)
 		klog.Errorf(msg)
 		setReadyCondition(newStatus, metav1.ConditionFalse, "FailedSelectResource", msg)
@@ -126,7 +125,7 @@ func (c *Controller) DoAnalytics(ctx context.Context, analytics *analysisv1alph1
 		recommendations, err = c.recommLister.Recommendations(analytics.Namespace).List(labels.Everything())
 	}
 	if err != nil {
-		c.Recorder.Event(analytics, v1.EventTypeNormal, "FailedSelectResource", err.Error())
+		c.Recorder.Event(analytics, corev1.EventTypeNormal, "FailedSelectResource", err.Error())
 		msg := fmt.Sprintf("Failed to get recomendations, Analytics %s error %v", klog.KObj(analytics), err)
 		klog.Errorf(msg)
 		setReadyCondition(newStatus, metav1.ConditionFalse, "FailedSelectResource", msg)
@@ -176,7 +175,7 @@ func (c *Controller) DoAnalytics(ctx context.Context, analytics *analysisv1alph1
 				rCopy := r.DeepCopy()
 				rCopy.OwnerReferences = append(rCopy.OwnerReferences, *newOwnerRef(analytics))
 				if err = c.Update(ctx, rCopy); err != nil {
-					c.Recorder.Event(analytics, v1.EventTypeNormal, "FailedUpdateRecommendation", err.Error())
+					c.Recorder.Event(analytics, corev1.EventTypeNormal, "FailedUpdateRecommendation", err.Error())
 					msg := fmt.Sprintf("Failed to update ownerReferences for recommendation %s, Analytics %s error %v", klog.KObj(rCopy), klog.KObj(analytics), err)
 					klog.Errorf(msg)
 					setReadyCondition(newStatus, metav1.ConditionFalse, "FailedUpdateRecommendation", msg)
@@ -187,7 +186,7 @@ func (c *Controller) DoAnalytics(ctx context.Context, analytics *analysisv1alph1
 			}
 		} else {
 			if err = c.CreateRecommendation(ctx, analytics, id, &refs); err != nil {
-				c.Recorder.Event(analytics, v1.EventTypeNormal, "FailedCreateRecommendation", err.Error())
+				c.Recorder.Event(analytics, corev1.EventTypeNormal, "FailedCreateRecommendation", err.Error())
 				msg := fmt.Sprintf("Failed to create recommendation, Analytics %s error %v", klog.KObj(analytics), err)
 				klog.Errorf(msg)
 				setReadyCondition(newStatus, metav1.ConditionFalse, "FailedCreateRecommendation", msg)
@@ -361,7 +360,7 @@ func (c *Controller) UpdateStatus(ctx context.Context, analytics *analysisv1alph
 		analytics.Status = *newStatus
 		err := c.Update(ctx, analytics)
 		if err != nil {
-			c.Recorder.Event(analytics, v1.EventTypeNormal, "FailedUpdateStatus", err.Error())
+			c.Recorder.Event(analytics, corev1.EventTypeNormal, "FailedUpdateStatus", err.Error())
 			klog.Errorf("Failed to update status, Analytics %s error %v", klog.KObj(analytics), err)
 			return
 		}

pkg/controller/ehpa/hpa.go

@@ -220,7 +220,7 @@ func (c *EffectiveHPAController) GetHPAMetrics(ctx context.Context, ehpa *autosc
 					}
 
 					if len(pods) == 0 {
-						return nil, fmt.Errorf("No pods returns from scale object. ")
+						return nil, fmt.Errorf("no pods returns from scale object. ")
 					}
 
 					requests, err := utils.CalculatePodRequests(pods, metric.Resource.Name)

pkg/controller/evpa/effective_vpa_controller.go

@@ -240,16 +240,16 @@ func defaultingEVPA(evpa *autoscalingapi.EffectiveVerticalPodAutoscaler) error {
 
 	for _, estimatorCurr := range evpa.Spec.ResourceEstimators {
 		if estimatorCurr.Type == "" {
-			return fmt.Errorf("Estimator type cannot be empty. ")
+			return fmt.Errorf("estimator type cannot be empty. ")
 		}
 	}
 
 	if evpa.Spec.ResourcePolicy == nil || len(evpa.Spec.ResourcePolicy.ContainerPolicies) == 0 {
-		return fmt.Errorf("Resource policy or container policy cannot be empty. ")
+		return fmt.Errorf("resource policy or container policy cannot be empty. ")
 	}
 	for index, containerPolicy := range evpa.Spec.ResourcePolicy.ContainerPolicies {
 		if containerPolicy.ContainerName == "" {
-			return fmt.Errorf("Container name cannot be empty. ")
+			return fmt.Errorf("container name cannot be empty. ")
 		}
 
 		// scale up

pkg/controller/recommendation/updater.go

@@ -48,7 +48,7 @@ func (c *Controller) UpdateRecommendation(ctx context.Context, recommendation *a
 	unstructed.SetKind(recommendation.Spec.TargetRef.Kind)
 	err := c.Client.Get(ctx, client.ObjectKey{Name: recommendation.Spec.TargetRef.Name, Namespace: recommendation.Spec.TargetRef.Namespace}, unstructed)
 	if err != nil {
-		return fmt.Errorf("Get target object failed: %v. ", err)
+		return fmt.Errorf("get target object failed: %v. ", err)
 	}
 
 	if recommendation.Spec.AdoptionType == analysisapi.AdoptionTypeStatusAndAnnotation || recommendation.Spec.AdoptionType == analysisapi.AdoptionTypeAuto {
@@ -67,7 +67,7 @@ func (c *Controller) UpdateRecommendation(ctx context.Context, recommendation *a
 		unstructed.SetAnnotations(annotation)
 		err = c.Client.Update(ctx, unstructed)
 		if err != nil {
-			return fmt.Errorf("Update target annotation failed: %v. ", err)
+			return fmt.Errorf("update target annotation failed: %v. ", err)
 		}
 	}
 
@@ -76,7 +76,7 @@ func (c *Controller) UpdateRecommendation(ctx context.Context, recommendation *a
 		if proposed.EffectiveHPA != nil {
 			ehpa, err := utils.GetEHPAFromScaleTarget(ctx, c.Client, recommendation.Spec.TargetRef.Namespace, recommendation.Spec.TargetRef)
 			if err != nil {
-				return fmt.Errorf("Get EHPA from target failed: %v. ", err)
+				return fmt.Errorf("get EHPA from target failed: %v. ", err)
 			}
 			if ehpa == nil {
 				ehpa = &autoscalingapi.EffectiveHorizontalPodAutoscaler{
@@ -127,7 +127,7 @@ func (c *Controller) UpdateRecommendation(ctx context.Context, recommendation *a
 		if proposed.ResourceRequest != nil {
 			evpa, err := utils.GetEVPAFromScaleTarget(ctx, c.Client, recommendation.Spec.TargetRef.Namespace, recommendation.Spec.TargetRef)
 			if err != nil {
-				return fmt.Errorf("Get EVPA from target failed: %v. ", err)
+				return fmt.Errorf("get EVPA from target failed: %v. ", err)
 			}
 			if evpa == nil {
 				off := vpatypes.UpdateModeOff

pkg/controller/timeseriesprediction/config.go

@@ -38,10 +38,11 @@ func NewMetricContext(fetcher target.SelectorFetcher, seriesPrediction *predicti
 		predictorMgr:     predictorMgr,
 		fetcher:          fetcher,
 	}
-	if strings.ToLower(c.TargetKind) != strings.ToLower(predconf.TargetKindNode) && seriesPrediction.Spec.TargetRef.Namespace != "" {
+	if !strings.EqualFold(c.TargetKind, predconf.TargetKindNode) && seriesPrediction.Spec.TargetRef.Namespace != "" {
 		c.Namespace = seriesPrediction.Spec.TargetRef.Namespace
 	}
-	if strings.ToLower(c.TargetKind) != strings.ToLower(predconf.TargetKindNode) {
+
+	if !strings.EqualFold(c.TargetKind, predconf.TargetKindNode) {
 		selector, err := c.fetcher.Fetch(&seriesPrediction.Spec.TargetRef)
 		if err != nil {
 			return nil, err
@@ -132,9 +133,7 @@ func metricSelectorToQueryExpr(m *predictionapi.MetricQuery) string {
 	conditions := make([]string, 0, len(m.QueryConditions))
 	for _, cond := range m.QueryConditions {
 		values := make([]string, 0, len(cond.Value))
-		for _, val := range cond.Value {
-			values = append(values, val)
-		}
+		values = append(values, cond.Value...)
 		sort.Strings(values)
 		conditions = append(conditions, fmt.Sprintf("%s%s[%s]", cond.Key, cond.Operator, strings.Join(values, ",")))
 	}
@@ -146,7 +145,7 @@ func (c *MetricContext) ResourceToMetricNamer(resourceName *corev1.ResourceName,
 	var namer metricnaming.GeneralMetricNamer
 
 	// Node
-	if strings.ToLower(c.TargetKind) == strings.ToLower(predconf.TargetKindNode) {
+	if strings.EqualFold(c.TargetKind, predconf.TargetKindNode) {
 		namer = metricnaming.GeneralMetricNamer{
 			CallerName: caller,
 			Metric: &metricquery.Metric{

pkg/controller/timeseriesprediction/status.go

@@ -229,8 +229,5 @@ func IsWindowInSamples(start, end time.Time, samples []predictionapi.Sample) boo
 	//startTs := start.Truncate(1 * time.Minute).Unix()
 	endTs := end.Truncate(1 * time.Minute).Unix()
 	// only check the end, start not check, because start is always from now to predict
-	if endTs <= samples[n-1].Timestamp {
-		return true
-	}
-	return false
+	return endTs <= samples[n-1].Timestamp
 }

pkg/ensurance/executor/schedule.go

@@ -93,7 +93,7 @@ func (b *ScheduleExecutor) Restore(ctx *ExecuteContext) error {
 
 func (p *ComparablePod) Less(p2 ComparablePod) bool {
 	if comparePodQos(p.Status.QOSClass, p2.Status.QOSClass) == 1 {
-
+		return false
 	}
 
 	return *p.Spec.Priority < *p2.Spec.Priority

pkg/ensurance/runtime/container.go

@@ -98,7 +98,7 @@ func UpdateContainerResources(client pb.RuntimeServiceClient, containerId string
 // copied from kubernetes-sigs/cri-tools/cmd/crictl/container.go
 func RemoveContainer(client pb.RuntimeServiceClient, ContainerId string) error {
 	if ContainerId == "" {
-		return fmt.Errorf("ID cannot be empty")
+		return fmt.Errorf("id cannot be empty")
 	}
 
 	request := &pb.RemoveContainerRequest{

pkg/metricprovider/custom_metric_provider.go

@@ -89,7 +89,7 @@ func (p *CustomMetricProvider) GetMetricBySelector(ctx context.Context, namespac
 
 	readyPods := GetReadyPods(pods)
 	if len(readyPods) == 0 {
-		return nil, fmt.Errorf("Failed to get ready pods. ")
+		return nil, fmt.Errorf("failed to get ready pods. ")
 	}
 
 	isPredicting := false
@@ -130,7 +130,7 @@ func (p *CustomMetricProvider) GetMetricBySelector(ctx context.Context, namespac
 
 		valueFloat, err := strconv.ParseFloat(v.Value, 32)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse value to float: %v ", err)
+			return nil, fmt.Errorf("failed to parse value to float: %v ", err)
 		}
 		if valueFloat > largestMetricValue.value {
 			largestMetricValue.value = valueFloat
@@ -223,9 +223,9 @@ func (p *CustomMetricProvider) GetPrediction(ctx context.Context, namespace stri
 	}
 	err = p.client.List(ctx, predictionList, opts...)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to get TimeSeriesPrediction when get custom metric ")
+		return nil, fmt.Errorf("failed to get TimeSeriesPrediction when get custom metric ")
 	} else if len(predictionList.Items) != 1 {
-		return nil, fmt.Errorf("Only one TimeSeriesPrediction should match the selector %s ", metricSelector.String())
+		return nil, fmt.Errorf("only one TimeSeriesPrediction should match the selector %s ", metricSelector.String())
 	}
 
 	return &predictionList.Items[0], nil
@@ -245,7 +245,7 @@ func (p *CustomMetricProvider) GetPods(ctx context.Context, namespace string, se
 	}
 	err = p.client.List(ctx, podList, opts...)
 	if err != nil || len(podList.Items) == 0 {
-		return nil, fmt.Errorf("Failed to get pods when get custom metric ")
+		return nil, fmt.Errorf("failed to get pods when get custom metric ")
 	}
 
 	return podList.Items, nil

pkg/metricprovider/remote.go

@@ -38,7 +38,7 @@ func NewRemoteAdapter(namespace string, name string, port int, config *rest.Conf
 
 	discoveryClientSet, err := discovery.NewDiscoveryClientForConfig(metricConfig)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to create discovery client: %v ", err)
+		return nil, fmt.Errorf("failed to create discovery client: %v ", err)
 	}
 
 	apiVersionsGetter := cmClient.NewAvailableAPIsGetter(discoveryClientSet)
@@ -94,7 +94,7 @@ func (p *RemoteAdapter) GetMetricByName(ctx context.Context, name types.Namespac
 
 	kind, err := utils.KindForResource(info.GroupResource.Resource, p.restMapper)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to get kind for resource %s: %v ", info.GroupResource.Resource, err)
+		return nil, fmt.Errorf("failed to get kind for resource %s: %v ", info.GroupResource.Resource, err)
 	}
 
 	var object *v1beta2.MetricValue
@@ -115,7 +115,7 @@ func (p *RemoteAdapter) GetMetricByName(ctx context.Context, name types.Namespac
 	}
 
 	if err != nil {
-		return nil, fmt.Errorf("Failed to get metric by name from remote: %v ", err)
+		return nil, fmt.Errorf("failed to get metric by name from remote: %v ", err)
 	}
 
 	return &custom_metrics.MetricValue{
@@ -142,7 +142,7 @@ func (p *RemoteAdapter) GetMetricBySelector(ctx context.Context, namespace strin
 
 	kind, err := utils.KindForResource(info.GroupResource.Resource, p.restMapper)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to get kind for resource %s: %v ", info.GroupResource.Resource, err)
+		return nil, fmt.Errorf("failed to get kind for resource %s: %v ", info.GroupResource.Resource, err)
 	}
 
 	var objects *v1beta2.MetricValueList
@@ -168,7 +168,7 @@ func (p *RemoteAdapter) GetMetricBySelector(ctx context.Context, namespace strin
 		)
 	}
 	if err != nil {
-		return nil, fmt.Errorf("Failed to get metric by selector from remote: %v ", err)
+		return nil, fmt.Errorf("failed to get metric by selector from remote: %v ", err)
 	}
 	values := make([]custom_metrics.MetricValue, len(objects.Items))
 	for i, v := range objects.Items {

pkg/metrics/ensuarance.go

@@ -201,12 +201,12 @@ func RegisterCraneAgent() {
 // UpdateDurationFromStart records the duration of the step identified by the
 // label using start time
 func UpdateDurationFromStart(module string, stepName StepLabel, start time.Time) {
-	duration := time.Now().Sub(start)
+	duration := time.Since(start)
 	UpdateDuration(module, stepName, duration)
 }
 
 func UpdateDurationFromStartWithSubComponent(module string, subComponent string, stepName StepLabel, start time.Time) {
-	duration := time.Now().Sub(start)
+	duration := time.Since(start)
 	UpdateDurationWithSubComponent(module, subComponent, stepName, duration)
 }
 

pkg/metrics/health.go

@@ -72,7 +72,7 @@ func (hc *HealthCheck) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if now.After(lastActivity.Add(hc.activityTimeout)) {
 		w.WriteHeader(http.StatusInternalServerError)
 
-		if _, err := w.Write([]byte(fmt.Sprintf("Error: last activity more %v ago", time.Now().Sub(lastActivity).String()))); err != nil {
+		if _, err := w.Write([]byte(fmt.Sprintf("Error: last activity more %v ago", time.Since(lastActivity).String()))); err != nil {
 			klog.Errorf("Http write failed: %v", err)
 			return
 		}
@@ -83,7 +83,7 @@ func (hc *HealthCheck) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if now.After(lastConfigUpdated.Add(hc.activityTimeout)) {
 		w.WriteHeader(http.StatusInternalServerError)
 
-		if _, err := w.Write([]byte(fmt.Sprintf("Error: last config update  more %v ago", time.Now().Sub(lastConfigUpdated).String()))); err != nil {
+		if _, err := w.Write([]byte(fmt.Sprintf("Error: last config update  more %v ago", time.Since(lastConfigUpdated).String()))); err != nil {
 			klog.Errorf("Http write failed: %v", err)
 			return
 		}

pkg/metrics/tsp_metric_collector.go

@@ -153,9 +153,7 @@ func metricSelectorToQueryExpr(m *predictionapi.MetricQuery) string {
 	conditions := make([]string, 0, len(m.QueryConditions))
 	for _, cond := range m.QueryConditions {
 		values := make([]string, 0, len(cond.Value))
-		for _, val := range cond.Value {
-			values = append(values, val)
-		}
+		values = append(values, cond.Value...)
 		sort.Strings(values)
 		conditions = append(conditions, fmt.Sprintf("%s%s[%s]", cond.Key, cond.Operator, strings.Join(values, ",")))
 	}

pkg/prediction/config/config.go

@@ -14,9 +14,7 @@ func metricSelectorToQueryExpr(m *predictionapi.MetricQuery) string {
 	conditions := make([]string, 0, len(m.QueryConditions))
 	for _, cond := range m.QueryConditions {
 		values := make([]string, 0, len(cond.Value))
-		for _, val := range cond.Value {
-			values = append(values, val)
-		}
+		values = append(values, cond.Value...)
 		sort.Strings(values)
 		conditions = append(conditions, fmt.Sprintf("%s%s[%s]", cond.Key, cond.Operator, strings.Join(values, ",")))
 	}