Improved support for empty command line arguments (#80)

* Don't add empty strings to arg values, support empty flags, tests

Signed-off-by: Ira <IRAR@il.ibm.com>

* Fixed lint error

Signed-off-by: Ira <IRAR@il.ibm.com>

---------

Signed-off-by: Ira <IRAR@il.ibm.com>

Author: irar2

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

@@ -125,7 +125,7 @@ func (c *configuration) validate() error {
 	// Upstream vLLM behaviour: when --served-model-name is not provided,
 	// it falls back to using the value of --model as the single public name
 	// returned by the API and exposed in Prometheus metrics.
-	if len(c.ServedModelNames) == 0 || c.ServedModelNames[0] == "" {
+	if len(c.ServedModelNames) == 0 {
 		c.ServedModelNames = []string{c.Model}
 	}
 

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

@@ -24,6 +24,8 @@ import (
 	"k8s.io/klog/v2"
 )
 
+const qwenModelName = "Qwen/Qwen2-0.5B"
+
 func createSimConfig(args []string) (*configuration, error) {
 	oldArgs := os.Args
 	defer func() {
@@ -66,7 +68,7 @@ var _ = Describe("Simulator configuration", func() {
 	// Config from config.yaml file
 	c = newConfig()
 	c.Port = 8001
-	c.Model = "Qwen/Qwen2-0.5B"
+	c.Model = qwenModelName
 	c.ServedModelNames = []string{"model1", "model2"}
 	c.MaxLoras = 2
 	c.MaxCPULoras = 5
@@ -128,7 +130,7 @@ var _ = Describe("Simulator configuration", func() {
 		"{\"name\":\"lora3\",\"path\":\"/path/to/lora3\"}",
 	}
 	test = testCase{
-		name: "config file with command line args",
+		name: "config file with command line args with different format",
 		args: []string{"cmd", "--model", model, "--config", "../../manifests/config.yaml", "--port", "8002",
 			"--served-model-name",
 			"--lora-modules={\"name\":\"lora3\",\"path\":\"/path/to/lora3\"}",
@@ -153,7 +155,7 @@ var _ = Describe("Simulator configuration", func() {
 		"{\"name\":\"lora3\",\"path\":\"/path/to/lora3\"}",
 	}
 	test = testCase{
-		name: "config file with command line args",
+		name: "config file with command line args with empty string",
 		args: []string{"cmd", "--model", model, "--config", "../../manifests/config.yaml", "--port", "8002",
 			"--served-model-name", "",
 			"--lora-modules", "{\"name\":\"lora3\",\"path\":\"/path/to/lora3\"}",
@@ -162,6 +164,44 @@ var _ = Describe("Simulator configuration", func() {
 	}
 	tests = append(tests, test)
 
+	// Config from config.yaml file plus command line args with empty string for loras
+	c = newConfig()
+	c.Port = 8001
+	c.Model = qwenModelName
+	c.ServedModelNames = []string{"model1", "model2"}
+	c.MaxLoras = 2
+	c.MaxCPULoras = 5
+	c.MaxNumSeqs = 5
+	c.TimeToFirstToken = 2
+	c.InterTokenLatency = 1
+	c.LoraModules = []loraModule{}
+	c.LoraModulesString = []string{}
+	test = testCase{
+		name:           "config file with command line args with empty string for loras",
+		args:           []string{"cmd", "--config", "../../manifests/config.yaml", "--lora-modules", ""},
+		expectedConfig: c,
+	}
+	tests = append(tests, test)
+
+	// Config from config.yaml file plus command line args with empty parameter for loras
+	c = newConfig()
+	c.Port = 8001
+	c.Model = qwenModelName
+	c.ServedModelNames = []string{"model1", "model2"}
+	c.MaxLoras = 2
+	c.MaxCPULoras = 5
+	c.MaxNumSeqs = 5
+	c.TimeToFirstToken = 2
+	c.InterTokenLatency = 1
+	c.LoraModules = []loraModule{}
+	c.LoraModulesString = []string{}
+	test = testCase{
+		name:           "config file with command line args with empty parameter for loras",
+		args:           []string{"cmd", "--config", "../../manifests/config.yaml", "--lora-modules"},
+		expectedConfig: c,
+	}
+	tests = append(tests, test)
+
 	// Invalid configurations
 	test = testCase{
 		name: "invalid model",
@@ -205,17 +245,19 @@ var _ = Describe("Simulator configuration", func() {
 		Entry(tests[2].name, tests[2].args, tests[2].expectedConfig),
 		Entry(tests[3].name, tests[3].args, tests[3].expectedConfig),
 		Entry(tests[4].name, tests[4].args, tests[4].expectedConfig),
+		Entry(tests[5].name, tests[5].args, tests[5].expectedConfig),
+		Entry(tests[6].name, tests[6].args, tests[6].expectedConfig),
 	)
 
 	DescribeTable("invalid configurations",
 		func(args []string) {
 			_, err := createSimConfig(args)
 			Expect(err).To(HaveOccurred())
 		},
-		Entry(tests[5].name, tests[5].args),
-		Entry(tests[6].name, tests[6].args),
 		Entry(tests[7].name, tests[7].args),
 		Entry(tests[8].name, tests[8].args),
 		Entry(tests[9].name, tests[9].args),
+		Entry(tests[10].name, tests[10].args),
+		Entry(tests[11].name, tests[11].args),
 	)
 })

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

@@ -143,7 +143,7 @@ func (s *VllmSimulator) parseCommandParamsAndLoadConfig() error {
 	servedModelNames := getParamValueFromArgs("served-model-name")
 	loraModuleNames := getParamValueFromArgs("lora-modules")
 
-	f := pflag.NewFlagSet("llm-d-inference-sim flags", pflag.ExitOnError)
+	f := pflag.NewFlagSet("llm-d-inference-sim flags", pflag.ContinueOnError)
 
 	f.IntVar(&config.Port, "port", config.Port, "Port")
 	f.StringVar(&config.Model, "model", config.Model, "Currently 'loaded' model")
@@ -157,12 +157,14 @@ func (s *VllmSimulator) parseCommandParamsAndLoadConfig() error {
 	f.Int64Var(&config.Seed, "seed", config.Seed, "Random seed for operations (if not set, current Unix time in nanoseconds is used)")
 
 	// These values were manually parsed above in getParamValueFromArgs, we leave this in order to get these flags in --help
-	var servedModelNameStrings multiString
-	f.Var(&servedModelNameStrings, "served-model-name", "Model names exposed by the API (a list of space-separated strings)")
-	var configFile string
-	f.StringVar(&configFile, "config", "", "The configuration file")
-	var loras multiString
-	f.Var(&loras, "lora-modules", "List of LoRA adapters (a list of space-separated JSON strings)")
+	var dummyString string
+	f.StringVar(&dummyString, "config", "", "The configuration file")
+	var dummyMultiString multiString
+	f.Var(&dummyMultiString, "served-model-name", "Model names exposed by the API (a list of space-separated strings)")
+	f.Var(&dummyMultiString, "lora-modules", "List of LoRA adapters (a list of space-separated JSON strings)")
+	// In order to allow empty arguments, we set a dummy NoOptDefVal for these flags
+	f.Lookup("served-model-name").NoOptDefVal = "dummy"
+	f.Lookup("lora-modules").NoOptDefVal = "dummy"
 
 	flagSet := flag.NewFlagSet("simFlagSet", flag.ExitOnError)
 	klog.InitFlags(flagSet)
@@ -208,7 +210,9 @@ func getParamValueFromArgs(param string) []string {
 			if strings.HasPrefix(arg, "--") {
 				break
 			}
-			values = append(values, arg)
+			if arg != "" {
+				values = append(values, arg)
+			}
 		} else {
 			if arg == "--"+param {
 				readValues = true