Skip to content

Fixed the rest of the lint errors and updating the linters #134

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

Merged
merged 1 commit into from
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/inferencemodel_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions pkg/ext-proc/backend/datastore.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package backend

import (
"fmt"
"errors"
"math/rand"
"sync"

Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/ext-proc/backend/endpointslice_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions pkg/ext-proc/backend/inferencepool_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions pkg/ext-proc/backend/vllm/metrics_test.go
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/ext-proc/handlers/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand Down
5 changes: 2 additions & 3 deletions pkg/ext-proc/handlers/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions pkg/ext-proc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
7 changes: 3 additions & 4 deletions pkg/ext-proc/scheduling/filter.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/ext-proc/scheduling/filter_test.go
Original file line number Diff line number Diff line change
@@ -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"
)

Expand All @@ -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,
},
Expand Down
3 changes: 1 addition & 2 deletions pkg/ext-proc/scheduling/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
3 changes: 1 addition & 2 deletions pkg/ext-proc/test/benchmark/benchmark.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
13 changes: 5 additions & 8 deletions pkg/ext-proc/test/hermetic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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()
Expand Down
13 changes: 8 additions & 5 deletions pkg/ext-proc/test/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down
Loading