fix: Lint errors (#20)

* Allow set imports of Ginkgo related packages

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* Updates suggested by golangci-lint

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* Changed golangci-lint config file format to V2

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* Install ginkgo for tests in ci-pr-checks workflow

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

---------

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

Author: shmuelk

.github/workflows/ci-pr-checks.yaml

@@ -30,4 +30,10 @@ jobs:
       - name: Run go test
         shell: bash 
         run: |
+          echo "Installing Ginkgo..."
+          go install github.com/onsi/ginkgo/ginkgo@latest
+          export PATH=$PATH:$(go env GOPATH)/bin
+          echo "Ginkgo installed:"
+          ginkgo version
+          echo "Running tests with Ginkgo..."
           make test

.golangci.yml

@@ -31,3 +31,12 @@ linters:
   - prealloc
   - unparam
   - unused
+  settings:
+    revive:
+      rules:
+        - name: dot-imports
+          arguments:
+            - allowedPackages:
+              - "github.com/onsi/ginkgo"
+              - "github.com/onsi/ginkgo/v2"
+              - "github.com/onsi/gomega"

pkg/llm-d-inference-sim/defs.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// Definitions of all sturctures used by vLLM simultor
+// Definitions of all structures used by vLLM simulator
 // Contains the main simulator class and all definitions related to request/response for all supported APIs
 package llmdinferencesim
 
@@ -30,7 +30,7 @@ import (
 const (
 	modeRandom        = "random"
 	modeEcho          = "echo"
-	chatComplIdPrefix = "chatcmpl-"
+	chatComplIDPrefix = "chatcmpl-"
 	stopFinishReason  = "stop"
 	roleAssistant     = "assistant"
 	roleUser          = "user"
@@ -54,8 +54,8 @@ type VllmSimulator struct {
 	loraAdaptors sync.Map
 	// maxLoras defines maximum number of loaded loras
 	maxLoras int
-	// maxLoras defines maximum number of loras to store in CPU memory
-	maxCpuLoras int
+	// maxCPULoras defines maximum number of loras to store in CPU memory
+	maxCPULoras int
 	// runningLoras is a collection of running loras, key of lora's name, value is number of requests using this lora
 	runningLoras sync.Map
 	// waitingLoras will represent collection of loras defined in requests in the queue - Not implemented yet
@@ -267,10 +267,10 @@ func (req textCompletionRequest) createResponseText(mode string) (string, error)
 
 // getLastUserMsg returns last message from this request's messages with user role,
 // if does not exist - returns an empty string
-func (r *chatCompletionRequest) getLastUserMsg() string {
-	for i := len(r.Messages) - 1; i >= 0; i-- {
-		if r.Messages[i].Role == roleUser {
-			return r.Messages[i].Content
+func (req *chatCompletionRequest) getLastUserMsg() string {
+	for i := len(req.Messages) - 1; i >= 0; i-- {
+		if req.Messages[i].Role == roleUser {
+			return req.Messages[i].Content
 		}
 	}
 

pkg/llm-d-inference-sim/metrics.go

@@ -115,7 +115,7 @@ func (s *VllmSimulator) setInitialPrometheusMetrics() {
 func (s *VllmSimulator) reportLoras() {
 	var loras []string
 
-	s.runningLoras.Range(func(key interface{}, value interface{}) bool {
+	s.runningLoras.Range(func(key interface{}, _ interface{}) bool {
 		if lora, ok := key.(string); ok {
 			loras = append(loras, lora)
 		}

pkg/llm-d-inference-sim/simulator.go

@@ -20,6 +20,7 @@ package llmdinferencesim
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net"
 	"os"
@@ -86,7 +87,7 @@ func (s *VllmSimulator) parseCommandParams() error {
 	var lorasStr string
 	f.StringVar(&lorasStr, "lora", "", "List of LoRA adapters, separated by comma")
 	f.IntVar(&s.maxLoras, "max-loras", 1, "Maximum number of LoRAs in a single batch")
-	f.IntVar(&s.maxCpuLoras, "max-cpu-loras", 0, "Maximum number of LoRAs to store in CPU memory")
+	f.IntVar(&s.maxCPULoras, "max-cpu-loras", 0, "Maximum number of LoRAs to store in CPU memory")
 	f.Int64Var(&s.maxRunningReqs, "max-running-requests", 5, "Maximum number of inference requests that could be processed at the same time (parameter to simulate requests waiting queue)")
 
 	if err := f.Parse(os.Args[1:]); err != nil {
@@ -100,7 +101,7 @@ func (s *VllmSimulator) parseCommandParams() error {
 
 	// validate parsed values
 	if s.model == "" {
-		return fmt.Errorf("model parameter is empty")
+		return errors.New("model parameter is empty")
 	}
 	if s.mode != modeEcho && s.mode != modeRandom {
 		return fmt.Errorf("invalid mode '%s', valid values are 'random' and 'echo'", s.mode)
@@ -109,20 +110,20 @@ func (s *VllmSimulator) parseCommandParams() error {
 		return fmt.Errorf("invalid port '%d'", s.port)
 	}
 	if s.interTokenLatency < 0 {
-		return fmt.Errorf("inter token latency cannot be negative")
+		return errors.New("inter token latency cannot be negative")
 	}
 	if s.timeToFirstToken < 0 {
-		return fmt.Errorf("time to first token cannot be negative")
+		return errors.New("time to first token cannot be negative")
 	}
 	if s.maxLoras < 1 {
-		return fmt.Errorf("max loras cannot be less than 1")
+		return errors.New("max loras cannot be less than 1")
 	}
-	if s.maxCpuLoras == 0 {
+	if s.maxCPULoras == 0 {
 		// max cpu loras by default is same as max loras
-		s.maxCpuLoras = s.maxLoras
+		s.maxCPULoras = s.maxLoras
 	}
-	if s.maxCpuLoras < 1 {
-		return fmt.Errorf("max CPU loras cannot be less than 1")
+	if s.maxCPULoras < 1 {
+		return errors.New("max CPU loras cannot be less than 1")
 	}
 
 	// just to suppress not used lint error for now
@@ -309,13 +310,13 @@ func (s *VllmSimulator) reqProcessingWorker(ctx context.Context, id int) {
 				intValue := 0
 
 				if !ok {
-					s.logger.Info("Create referense counter", "model", model)
+					s.logger.Info("Create reference counter", "model", model)
 					intValue = 0
 				} else {
 					intValue = value.(int)
 				}
 				s.runningLoras.Store(model, intValue+1)
-				s.logger.Info("Update LoRA referense counter", "model", model, "old value", intValue, "new value", intValue+1)
+				s.logger.Info("Update LoRA reference counter", "model", model, "old value", intValue, "new value", intValue+1)
 
 				// TODO - check if thie request went to the waiting queue - add it to waiting map
 				s.reportLoras()
@@ -359,13 +360,13 @@ func (s *VllmSimulator) responseSentCallback(model string) {
 	value, ok := s.runningLoras.Load(model)
 
 	if !ok {
-		s.logger.Info("Error: nil referense counter", "model", model)
+		s.logger.Info("Error: nil reference counter", "model", model)
 		s.logger.Error(nil, "Zerro model reference", "model", model)
 	} else {
 		intValue := value.(int)
 		if intValue > 1 {
 			s.runningLoras.Store(model, intValue-1)
-			s.logger.Info("Update LoRA referense counter", "model", model, "prev value", intValue, "new value", intValue-1)
+			s.logger.Info("Update LoRA reference counter", "model", model, "prev value", intValue, "new value", intValue-1)
 		} else {
 			// last lora instance stopped it execution - remove from the map
 			s.runningLoras.Delete(model)
@@ -377,7 +378,7 @@ func (s *VllmSimulator) responseSentCallback(model string) {
 
 }
 
-// sendCompletionError sends an error response for the curent completion request
+// sendCompletionError sends an error response for the current completion request
 func (s *VllmSimulator) sendCompletionError(ctx *fasthttp.RequestCtx, msg string, errType string, code int) {
 	compErr := completionError{
 		Object:  "error",
@@ -400,12 +401,7 @@ func (s *VllmSimulator) sendCompletionError(ctx *fasthttp.RequestCtx, msg string
 
 // HandleModels handles /v1/models request according the data stored in the simulator
 func (s *VllmSimulator) HandleModels(ctx *fasthttp.RequestCtx) {
-	modelsResp, err := s.createModelsResponse()
-	if err != nil {
-		s.logger.Error(err, "Cannot create /models response")
-		ctx.Error("Cannot create /models response, "+err.Error(), fasthttp.StatusInternalServerError)
-		return
-	}
+	modelsResp := s.createModelsResponse()
 
 	data, err := json.Marshal(modelsResp)
 	if err != nil {
@@ -419,7 +415,7 @@ func (s *VllmSimulator) HandleModels(ctx *fasthttp.RequestCtx) {
 	ctx.Response.SetBody(data)
 }
 
-func (s *VllmSimulator) HandleError(ctx *fasthttp.RequestCtx, err error) {
+func (s *VllmSimulator) HandleError(_ *fasthttp.RequestCtx, err error) {
 	s.logger.Error(err, "VLLM server error")
 }
 
@@ -429,7 +425,7 @@ func (s *VllmSimulator) HandleError(ctx *fasthttp.RequestCtx, err error) {
 // finishReason - a pointer to string that represents finish reason, can be nil or stop or length, ...
 func (s *VllmSimulator) createCompletionResponse(isChatCompletion bool, respText string, model string, finishReason *string) completionResponse {
 	baseResp := baseCompletionResponse{
-		ID:      chatComplIdPrefix + uuid.NewString(),
+		ID:      chatComplIDPrefix + uuid.NewString(),
 		Created: time.Now().Unix(),
 		Model:   model,
 	}
@@ -440,11 +436,11 @@ func (s *VllmSimulator) createCompletionResponse(isChatCompletion bool, respText
 			baseCompletionResponse: baseResp,
 			Choices:                []chatRespChoice{{Message: message{Role: roleAssistant, Content: respText}, baseResponseChoice: baseChoice}},
 		}
-	} else {
-		return &textCompletionResponse{
-			baseCompletionResponse: baseResp,
-			Choices:                []textRespChoice{{baseResponseChoice: baseChoice, Text: respText}},
-		}
+	}
+
+	return &textCompletionResponse{
+		baseCompletionResponse: baseResp,
+		Choices:                []textRespChoice{{baseResponseChoice: baseChoice, Text: respText}},
 	}
 }
 
@@ -475,7 +471,7 @@ func (s *VllmSimulator) sendResponse(isChatCompletion bool, ctx *fasthttp.Reques
 }
 
 // createModelsResponse creates and returns ModelResponse for the current state, returned array of models contains the base model + LoRA adapters if exist
-func (s *VllmSimulator) createModelsResponse() (*vllmapi.ModelsResponse, error) {
+func (s *VllmSimulator) createModelsResponse() *vllmapi.ModelsResponse {
 	modelsResp := vllmapi.ModelsResponse{Object: "list", Data: []vllmapi.ModelsResponseModelInfo{}}
 
 	// add base model's info
@@ -500,5 +496,5 @@ func (s *VllmSimulator) createModelsResponse() (*vllmapi.ModelsResponse, error)
 		})
 	}
 
-	return &modelsResp, nil
+	return &modelsResp
 }

pkg/llm-d-inference-sim/simulator_test.go

@@ -69,7 +69,7 @@ func startServer(ctx context.Context, mode string) (*http.Client, error) {
 
 	return &http.Client{
 		Transport: &http.Transport{
-			DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+			DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
 				return listener.Dial()
 			},
 		},
@@ -123,7 +123,7 @@ var _ = Describe("Simulator", func() {
 			Expect(msg).Should(Equal(expectedMsg))
 		},
 		func(mode string) string {
-			return fmt.Sprintf("mode: %s", mode)
+			return "mode: " + mode
 		},
 		Entry(nil, modeRandom),
 		Entry(nil, modeEcho),
@@ -170,7 +170,7 @@ var _ = Describe("Simulator", func() {
 			Expect(text).Should(Equal(expectedText))
 		},
 		func(mode string) string {
-			return fmt.Sprintf("mode: %s", mode)
+			return "mode: " + mode
 		},
 		Entry(nil, modeRandom),
 		Entry(nil, modeEcho),
@@ -192,18 +192,18 @@ var _ = Describe("Simulator", func() {
 				},
 				Model: model,
 			}
-			num_tokens := 0
+			numTokens := 0
 			partialErrMsg := ""
 			// if maxTokens and maxCompletionTokens are passsed
 			// maxCompletionTokens is used
 			if maxTokens != 0 {
 				params.MaxTokens = param.NewOpt(int64(maxTokens))
-				num_tokens = maxTokens
+				numTokens = maxTokens
 				partialErrMsg = "max_tokens must be at least 1, got -1"
 			}
 			if maxCompletionTokens != 0 {
 				params.MaxCompletionTokens = param.NewOpt(int64(maxCompletionTokens))
-				num_tokens = maxCompletionTokens
+				numTokens = maxCompletionTokens
 				partialErrMsg = "max_completion_tokens must be at least 1, got -1"
 			}
 			resp, err := openaiclient.Chat.Completions.New(ctx, params)
@@ -220,14 +220,14 @@ var _ = Describe("Simulator", func() {
 				}
 			}
 			Expect(err).NotTo(HaveOccurred())
-			Expect(len(resp.Choices)).Should(BeNumerically(">", 0))
+			Expect(resp.Choices).ShouldNot(BeEmpty())
 
 			msg := resp.Choices[0].Message.Content
 			Expect(msg).ShouldNot(BeEmpty())
 
-			if num_tokens > 0 {
+			if numTokens > 0 {
 				tokens := strings.Fields(msg)
-				Expect(int64(len(tokens))).Should(BeNumerically("<=", num_tokens))
+				Expect(int64(len(tokens))).Should(BeNumerically("<=", numTokens))
 			} else {
 				expectedMsg := ""
 				if mode == modeEcho {
@@ -275,11 +275,11 @@ var _ = Describe("Simulator", func() {
 				},
 				Model: openai.CompletionNewParamsModel(model),
 			}
-			num_tokens := 0
+			numTokens := 0
 			partialErrMsg := "max_tokens must be at least 1, got -1"
 			if maxTokens != 0 {
 				params.MaxTokens = param.NewOpt(int64(maxTokens))
-				num_tokens = maxTokens
+				numTokens = maxTokens
 			}
 			resp, err := openaiclient.Completions.New(ctx, params)
 			if err != nil {
@@ -295,14 +295,14 @@ var _ = Describe("Simulator", func() {
 				}
 			}
 			Expect(err).NotTo(HaveOccurred())
-			Expect(len(resp.Choices)).Should(BeNumerically(">", 0))
+			Expect(resp.Choices).ShouldNot(BeEmpty())
 
 			text := resp.Choices[0].Text
 			Expect(text).ShouldNot(BeEmpty())
 
-			if num_tokens != 0 {
+			if numTokens != 0 {
 				tokens := strings.Fields(text)
-				Expect(int64(len(tokens))).Should(BeNumerically("<=", num_tokens))
+				Expect(int64(len(tokens))).Should(BeNumerically("<=", numTokens))
 			} else {
 				expectedText := ""
 				if mode == modeEcho {

pkg/llm-d-inference-sim/streaming.go

@@ -102,7 +102,7 @@ func (s *VllmSimulator) sendStreamingResponse(isChatCompletion bool, ctx *fastht
 // finishReason - a pointer to string that represents finish reason, can be nil or stop or length, ...
 func (s *VllmSimulator) createCompletionChunk(isChatCompletion bool, creationTime int64, token string, model string, role string, finishReason *string) completionRespChunk {
 	baseChunk := baseCompletionResponse{
-		ID:      chatComplIdPrefix + uuid.NewString(),
+		ID:      chatComplIDPrefix + uuid.NewString(),
 		Created: creationTime,
 		Model:   model,
 	}
@@ -122,11 +122,11 @@ func (s *VllmSimulator) createCompletionChunk(isChatCompletion bool, creationTim
 		}
 
 		return &chunk
-	} else {
-		return &textCompletionResponse{
-			baseCompletionResponse: baseChunk,
-			Choices:                []textRespChoice{{baseResponseChoice: baseChoice, Text: token}},
-		}
+	}
+
+	return &textCompletionResponse{
+		baseCompletionResponse: baseChunk,
+		Choices:                []textRespChoice{{baseResponseChoice: baseChoice, Text: token}},
 	}
 }