Fix preempt reclaim minruntime string parsing (#278)

* using str2duration lib to parse min runtime duration to support more cases

* validate the duration is not negative

* added tests for parseMinRuntime

* little fix in test

Author: natasharomm

go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/run-ai/kwok-operator v0.0.0-20240926063032-05b6364bc7c7
 	github.com/spf13/pflag v1.0.6
 	github.com/stretchr/testify v1.10.0
+	github.com/xhit/go-str2duration/v2 v2.1.0
 	github.com/xyproto/randomstring v1.2.0
 	go.uber.org/mock v0.5.0
 	go.uber.org/multierr v1.11.0

go.sum

@@ -327,6 +327,8 @@ github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65E
 github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg=
 github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
 github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
+github.com/xhit/go-str2duration/v2 v2.1.0 h1:lxklc02Drh6ynqX+DdPyp5pCKLUQpRT8bp8Ydu2Bstc=
+github.com/xhit/go-str2duration/v2 v2.1.0/go.mod h1:ohY8p+0f07DiV6Em5LKB0s2YpLtXVyJfNt1+BlmyAsU=
 github.com/xiang90/probing v0.0.0-20221125231312-a49e3df8f510 h1:S2dVYn90KE98chqDkyE9Z4N61UnQd+KOfgp5Iu53llk=
 github.com/xiang90/probing v0.0.0-20221125231312-a49e3df8f510/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/xyproto/randomstring v1.2.0 h1:y7PXAEBM3XlwJjPG2JQg4voxBYZ4+hPgRdGKCfU8wik=

pkg/scheduler/plugins/minruntime/minruntime.go

@@ -4,6 +4,7 @@
 package minruntime
 
 import (
+	"github.com/xhit/go-str2duration/v2"
 	"slices"
 	"time"
 
@@ -41,11 +42,17 @@ func parseMinRuntime(arguments map[string]string, minRuntimeConfig string) metav
 	if len(minRuntime) == 0 {
 		return metav1.Duration{Duration: 0 * time.Second}
 	}
-	duration, err := time.ParseDuration(minRuntime)
+	duration, err := str2duration.ParseDuration(minRuntime)
 	if err != nil {
 		log.InfraLogger.Errorf("Failed to parse %v (%v): %v, using default value 0s", minRuntimeConfig, minRuntime, err)
 		duration = 0 * time.Second
 	}
+
+	if duration < 0 {
+		log.InfraLogger.Errorf("Parsed %v (%v) is negative, using default value 0s", minRuntimeConfig, minRuntime)
+		duration = 0 * time.Second
+	}
+
 	return metav1.Duration{Duration: duration}
 }
 

pkg/scheduler/plugins/minruntime/minruntime_test.go

@@ -333,4 +333,51 @@ var _ = Describe("MinRuntime Plugin", func() {
 			})
 		})
 	})
+
+	Describe("parseMinRuntime", func() {
+		It("Sanity - parse passes", func() {
+			// Test various valid min runtime strings
+			validDurations := []string{
+				"5s", "10s", "10m5s", "1m", "1.5s", "2m30s", "1h", "1h30m", "2h45m15s", "2d4h30m", "5w4d12h",
+			}
+
+			args := map[string]string{}
+
+			for _, durationStr := range validDurations {
+				args[defaultReclaimMinRuntimeConfig] = durationStr
+				duration := parseMinRuntime(args, defaultReclaimMinRuntimeConfig)
+				Expect(duration).ToNot(BeZero(), fmt.Sprintf("Expected non-zero duration for %s", durationStr))
+			}
+		})
+
+		It("Invalid durations should return zero", func() {
+			// Test various invalid min runtime strings
+			invalidDurations := []string{
+				"5", "1h2", "2h45m15", "abc", "1h-30m", "dfdsfdfdf",
+			}
+
+			args := map[string]string{}
+
+			for _, durationStr := range invalidDurations {
+				args[defaultPreemptMinRuntimeConfig] = durationStr
+				duration := parseMinRuntime(args, defaultPreemptMinRuntimeConfig)
+				Expect(duration).To(BeZero(), fmt.Sprintf("Expected zero duration for invalid %s", durationStr))
+			}
+		})
+
+		It("Invalid negative durations", func() {
+			// Test negative durations
+			negativeDurations := []string{
+				"-5s", "-10m", "-1h", "-2d", "-3w",
+			}
+
+			args := map[string]string{}
+
+			for _, durationStr := range negativeDurations {
+				args[defaultReclaimMinRuntimeConfig] = durationStr
+				duration := parseMinRuntime(args, defaultReclaimMinRuntimeConfig)
+				Expect(duration).To(BeZero(), fmt.Sprintf("Expected zero duration for negative %s", durationStr))
+			}
+		})
+	})
 })