Fixed the rest of the lint errors and updating the linters to match what is typically used in other sub-projects (#134)

ahg-g · web-flow · commit f34e7f680d77 · 2024-12-26T19:00:12.000+01:00
diff --git a/.golangci.yml b/.golangci.yml
@@ -2,45 +2,41 @@ run:
   timeout: 5m
   allow-parallel-runners: true
 
+# Settings related to issues
 issues:
-  # don't skip warning about doc comments
-  # don't exclude the default set of lint
-  exclude-use-default: false
-  # restore some of the defaults
-  # (fill in the rest as needed)
-  exclude-rules:
-    - path: "api/*"
-      linters:
-        - lll
-    - path: "internal/*"
-      linters:
-        - dupl
-        - lll
+  # Which dirs to exclude: issues from them won't be reported
+  exclude-dirs:
+    - bin
 linters:
   disable-all: true
   enable:
-    - dupl
-    - errcheck
     - copyloopvar
+    - dupword
+    - durationcheck
+    - fatcontext
+    - gci
     - ginkgolinter
+    - gocritic
+    - govet
+    - loggercheck
+    - misspell
+    - perfsprint
+    - revive
+    - unconvert
+    - makezero
+    - errcheck
     - goconst
     - gocyclo
     - gofmt
     - goimports
     - gosimple
-    - govet
     - ineffassign
-    - lll
-    - misspell
     - nakedret
     - prealloc
-    - revive
-    - staticcheck
     - typecheck
-    - unconvert
     - unparam
     - unused
-
+    
 linters-settings:
   revive:
     rules:
diff --git a/Makefile b/Makefile
@@ -122,7 +122,7 @@ ci-lint: golangci-lint
 	$(GOLANGCI_LINT) run --timeout 15m0s
 
 .PHONY: verify
-verify: vet fmt-verify manifests generate ## ci-lint add back when all lint errors are fixed
+verify: vet fmt-verify manifests generate ci-lint
 	git --no-pager diff --exit-code config api client-go
 
 ##@ Build
diff --git a/api/v1alpha1/inferencemodel_types.go b/api/v1alpha1/inferencemodel_types.go
@@ -41,7 +41,7 @@ type InferenceModelSpec struct {
 	// The name of the model as the users set in the "model" parameter in the requests.
 	// The name should be unique among the workloads that reference the same backend pool.
 	// This is the parameter that will be used to match the request with. In the future, we may
-	// allow to match on other request parameters. The other approach to support matching on
+	// allow to match on other request parameters. The other approach to support matching
 	// on other request parameters is to use a different ModelName per HTTPFilter.
 	// Names can be reserved without implementing an actual model in the pool.
 	// This can be done by specifying a target model and setting the weight to zero,
diff --git a/config/crd/bases/inference.networking.x-k8s.io_inferencemodels.yaml b/config/crd/bases/inference.networking.x-k8s.io_inferencemodels.yaml
@@ -68,7 +68,7 @@ spec:
                   The name of the model as the users set in the "model" parameter in the requests.
                   The name should be unique among the workloads that reference the same backend pool.
                   This is the parameter that will be used to match the request with. In the future, we may
-                  allow to match on other request parameters. The other approach to support matching on
+                  allow to match on other request parameters. The other approach to support matching
                   on other request parameters is to use a different ModelName per HTTPFilter.
                   Names can be reserved without implementing an actual model in the pool.
                   This can be done by specifying a target model and setting the weight to zero,
diff --git a/pkg/ext-proc/backend/datastore.go b/pkg/ext-proc/backend/datastore.go
@@ -1,7 +1,7 @@
 package backend
 
 import (
-	"fmt"
+	"errors"
 	"math/rand"
 	"sync"
 
@@ -53,7 +53,7 @@ func (ds *K8sDatastore) getInferencePool() (*v1alpha1.InferencePool, error) {
 	ds.poolMu.RLock()
 	defer ds.poolMu.RUnlock()
 	if ds.inferencePool == nil {
-		return nil, fmt.Errorf("InferencePool hasn't been initialized yet")
+		return nil, errors.New("InferencePool hasn't been initialized yet")
 	}
 	return ds.inferencePool, nil
 }
diff --git a/pkg/ext-proc/backend/endpointslice_reconciler.go b/pkg/ext-proc/backend/endpointslice_reconciler.go
@@ -2,7 +2,7 @@ package backend
 
 import (
 	"context"
-	"fmt"
+	"strconv"
 
 	"inference.networking.x-k8s.io/llm-instance-gateway/api/v1alpha1"
 	discoveryv1 "k8s.io/api/discovery/v1"
@@ -57,7 +57,7 @@ func (c *EndpointSliceReconciler) updateDatastore(
 		if c.validPod(endpoint) {
 			pod := Pod{
 				Name:    endpoint.TargetRef.Name,
-				Address: endpoint.Addresses[0] + ":" + fmt.Sprint(inferencePool.Spec.TargetPortNumber),
+				Address: endpoint.Addresses[0] + ":" + strconv.Itoa(int(inferencePool.Spec.TargetPortNumber)),
 			}
 			podMap[pod] = true
 			c.Datastore.pods.Store(pod, true)
diff --git a/pkg/ext-proc/backend/inferencepool_reconciler.go b/pkg/ext-proc/backend/inferencepool_reconciler.go
@@ -3,13 +3,12 @@ package backend
 import (
 	"context"
 
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
 	"inference.networking.x-k8s.io/llm-instance-gateway/api/v1alpha1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // InferencePoolReconciler utilizes the controller runtime to reconcile Instance Gateway resources
diff --git a/pkg/ext-proc/backend/vllm/metrics_test.go b/pkg/ext-proc/backend/vllm/metrics_test.go
@@ -1,14 +1,13 @@
 package vllm
 
 import (
-	"fmt"
+	"errors"
 	"testing"
 
-	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
-
 	dto "github.com/prometheus/client_model/go"
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/protobuf/proto"
+	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
 )
 
 func TestPromToPodMetrics(t *testing.T) {
@@ -215,7 +214,7 @@ func TestPromToPodMetrics(t *testing.T) {
 				MaxActiveModels: 0,
 			},
 			initialPodMetrics: &backend.PodMetrics{},
-			expectedErr:       fmt.Errorf("strconv.Atoi: parsing '2a': invalid syntax"),
+			expectedErr:       errors.New("strconv.Atoi: parsing '2a': invalid syntax"),
 		},
 	}
 	for _, tc := range testCases {
diff --git a/pkg/ext-proc/handlers/request.go b/pkg/ext-proc/handlers/request.go
@@ -2,15 +2,15 @@ package handlers
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strconv"
 
 	configPb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	extProcPb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
-	klog "k8s.io/klog/v2"
-
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/scheduling"
+	klog "k8s.io/klog/v2"
 )
 
 // HandleRequestBody handles body of the request to the backend server, such as parsing the "model"
@@ -31,7 +31,7 @@ func (s *Server) HandleRequestBody(reqCtx *RequestContext, req *extProcPb.Proces
 	// Resolve target models.
 	model, ok := rb["model"].(string)
 	if !ok {
-		return nil, fmt.Errorf("model not found in request")
+		return nil, errors.New("model not found in request")
 	}
 	klog.V(3).Infof("Model requested: %v", model)
 	modelName := model
@@ -87,7 +87,7 @@ func (s *Server) HandleRequestBody(reqCtx *RequestContext, req *extProcPb.Proces
 			},
 		},
 		// We need to update the content length header if the body is mutated, see Envoy doc:
-		// https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/ext_proc/v3/processing_mode.proto#enum-extensions-filters-http-ext-proc-v3-processingmode-bodysendmode
+		// https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/ext_proc/v3/processing_mode.proto
 		{
 			Header: &configPb.HeaderValue{
 				Key:      "Content-Length",
diff --git a/pkg/ext-proc/handlers/server.go b/pkg/ext-proc/handlers/server.go
@@ -7,11 +7,10 @@ import (
 	envoyTypePb "github.com/envoyproxy/go-control-plane/envoy/type/v3"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
-	klog "k8s.io/klog/v2"
-
 	"inference.networking.x-k8s.io/llm-instance-gateway/api/v1alpha1"
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/scheduling"
+	klog "k8s.io/klog/v2"
 )
 
 func NewServer(pp PodProvider, scheduler Scheduler, targetPodHeader string, datastore ModelDataStore) *Server {
@@ -73,7 +72,7 @@ func (s *Server) Process(srv extProcPb.ExternalProcessor_ProcessServer) error {
 			return status.Errorf(codes.Unknown, "cannot receive stream request: %v", err)
 		}
 
-		resp := &extProcPb.ProcessingResponse{}
+		var resp *extProcPb.ProcessingResponse
 		switch v := req.Request.(type) {
 		case *extProcPb.ProcessingRequest_RequestHeaders:
 			resp = HandleRequestHeaders(reqCtx, req)
diff --git a/pkg/ext-proc/main.go b/pkg/ext-proc/main.go
@@ -16,16 +16,15 @@ import (
 	healthPb "google.golang.org/grpc/health/grpc_health_v1"
 	"google.golang.org/grpc/status"
 	"inference.networking.x-k8s.io/llm-instance-gateway/api/v1alpha1"
+	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
+	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend/vllm"
+	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/handlers"
+	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/scheduling"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	klog "k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
-
-	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
-	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend/vllm"
-	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/handlers"
-	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/scheduling"
 )
 
 var (
diff --git a/pkg/ext-proc/scheduling/filter.go b/pkg/ext-proc/scheduling/filter.go
@@ -1,12 +1,11 @@
 package scheduling
 
 import (
-	"fmt"
+	"errors"
 	"math"
 
-	klog "k8s.io/klog/v2"
-
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
+	klog "k8s.io/klog/v2"
 )
 
 type Filter interface {
@@ -86,7 +85,7 @@ func toFilterFunc(pp podPredicate) filterFunc {
 			}
 		}
 		if len(filtered) == 0 {
-			return nil, fmt.Errorf("no pods left")
+			return nil, errors.New("no pods left")
 		}
 		return filtered, nil
 	}
diff --git a/pkg/ext-proc/scheduling/filter_test.go b/pkg/ext-proc/scheduling/filter_test.go
@@ -1,11 +1,10 @@
 package scheduling
 
 import (
-	"fmt"
+	"errors"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
 )
 
@@ -21,7 +20,7 @@ func TestFilter(t *testing.T) {
 		{
 			name: "simple filter without successor, failure",
 			filter: &filter{filter: func(req *LLMRequest, pods []*backend.PodMetrics) ([]*backend.PodMetrics, error) {
-				return nil, fmt.Errorf("filter error")
+				return nil, errors.New("filter error")
 			}},
 			err: true,
 		},
diff --git a/pkg/ext-proc/scheduling/scheduler.go b/pkg/ext-proc/scheduling/scheduler.go
@@ -7,9 +7,8 @@ import (
 
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
-	klog "k8s.io/klog/v2"
-
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
+	klog "k8s.io/klog/v2"
 )
 
 const (
diff --git a/pkg/ext-proc/test/benchmark/benchmark.go b/pkg/ext-proc/test/benchmark/benchmark.go
@@ -10,11 +10,10 @@ import (
 	"github.com/bojand/ghz/runner"
 	"github.com/jhump/protoreflect/desc"
 	"google.golang.org/protobuf/proto"
-	klog "k8s.io/klog/v2"
-
 	"inference.networking.x-k8s.io/llm-instance-gateway/api/v1alpha1"
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/test"
+	klog "k8s.io/klog/v2"
 )
 
 var (
diff --git a/pkg/ext-proc/test/hermetic_test.go b/pkg/ext-proc/test/hermetic_test.go
@@ -4,20 +4,17 @@ package test
 import (
 	"context"
 	"fmt"
-	"log"
 	"testing"
 	"time"
 
-	"inference.networking.x-k8s.io/llm-instance-gateway/api/v1alpha1"
-	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
-
 	configPb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	extProcPb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
 	"github.com/google/go-cmp/cmp"
-	"google.golang.org/protobuf/testing/protocmp"
-
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/protobuf/testing/protocmp"
+	"inference.networking.x-k8s.io/llm-instance-gateway/api/v1alpha1"
+	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
 )
 
 const (
@@ -145,13 +142,13 @@ func setUpServer(t *testing.T, pods []*backend.PodMetrics, models map[string]*v1
 	// Create a grpc connection
 	conn, err := grpc.NewClient(address, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
-		log.Fatalf("Failed to connect to %v: %v", address, err)
+		t.Fatalf("Failed to connect to %v: %v", address, err)
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	client, err = extProcPb.NewExternalProcessorClient(conn).Process(ctx)
 	if err != nil {
-		log.Fatalf("Failed to create client: %v", err)
+		t.Fatalf("Failed to create client: %v", err)
 	}
 	return client, func() {
 		cancel()
diff --git a/pkg/ext-proc/test/utils.go b/pkg/ext-proc/test/utils.go
@@ -6,16 +6,14 @@ import (
 	"net"
 	"time"
 
+	extProcPb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/reflection"
-	klog "k8s.io/klog/v2"
-
-	extProcPb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
-
 	"inference.networking.x-k8s.io/llm-instance-gateway/api/v1alpha1"
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/backend"
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/handlers"
 	"inference.networking.x-k8s.io/llm-instance-gateway/pkg/ext-proc/scheduling"
+	klog "k8s.io/klog/v2"
 )
 
 func StartExtProc(port int, refreshPodsInterval, refreshMetricsInterval time.Duration, pods []*backend.PodMetrics, models map[string]*v1alpha1.InferenceModel) *grpc.Server {
@@ -46,7 +44,12 @@ func startExtProc(port int, pp *backend.Provider, models map[string]*v1alpha1.In
 
 	klog.Infof("Starting gRPC server on port :%v", port)
 	reflection.Register(s)
-	go s.Serve(lis)
+	go func() {
+		err := s.Serve(lis)
+		if err != nil {
+			klog.Fatalf("Ext-proc failed with the err: %v", err)
+		}
+	}()
 	return s
 }
 
diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go
diff --git a/test/utils/utils.go b/test/utils/utils.go
