fix(scheduler): honour HAMI_NODELOCK_EXPIRE when the flag is not set

cmd/scheduler/main.go

@@ -76,7 +76,10 @@ func init() {
 	rootCmd.Flags().IntVar(&config.Burst, "kube-burst", client.DefaultBurst, "Burst to use while talking with kube-apiserver.")
 	rootCmd.Flags().IntVar(&config.Timeout, "kube-timeout", client.DefaultTimeout, "Timeout to use while talking with kube-apiserver.")
 	rootCmd.Flags().BoolVar(&enableProfiling, "profiling", false, "Enable pprof profiling via HTTP server")
-	rootCmd.Flags().DurationVar(&config.NodeLockTimeout, "node-lock-timeout", time.Minute*5, "timeout for node locks")
+	// nodelock's package init runs before this one and has already applied
+	// HAMI_NODELOCK_EXPIRE, so taking its value as the flag default leaves
+	// precedence as flag, then environment, then default.
+	rootCmd.Flags().DurationVar(&config.NodeLockTimeout, "node-lock-timeout", nodelock.NodeLockTimeout, "timeout for node locks")
 	rootCmd.Flags().DurationVar(&config.NodeLockRetryTimeout, "node-lock-retry-timeout", 28*time.Second, "timeout for retrying LockNode when contended by another PodGroup member (0 disables retry). Align the Extender's httpTimeout in KubeSchedulerConfiguration with this value.")
 	rootCmd.Flags().BoolVar(&config.ForceOverwriteDefaultScheduler, "force-overwrite-default-scheduler", true, "Overwrite schedulerName in Pod Spec when set to the const DefaultSchedulerName in https://k8s.io/api/core/v1 package")
 
@@ -110,7 +113,6 @@ func injectProfilingRoute(router *httprouter.Router) {
 }
 
 func start() error {
-	// Initialize node lock timeout from config
 	nodelock.NodeLockTimeout = config.NodeLockTimeout
 	klog.InfoS("Set node lock timeout", "timeout", nodelock.NodeLockTimeout)
 	client.InitGlobalClient(

cmd/scheduler/nodelock_timeout_test.go

@@ -0,0 +1,42 @@
+/*
+Copyright 2024 The HAMi Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"testing"
+
+	"github.com/Project-HAMi/HAMi/pkg/scheduler/config"
+	"github.com/Project-HAMi/HAMi/pkg/util/nodelock"
+)
+
+// TestNodeLockTimeoutFlagDefault guards the precedence of flag, then
+// environment, then default. nodelock's init applies HAMI_NODELOCK_EXPIRE
+// before this package's init registers flags, so the flag default must be
+// nodelock.NodeLockTimeout. Hard-coding a duration here instead would discard
+// the environment value on every start, which is the bug in #2692.
+func TestNodeLockTimeoutFlagDefault(t *testing.T) {
+	f := rootCmd.Flags().Lookup("node-lock-timeout")
+	if f == nil {
+		t.Fatal("node-lock-timeout flag is not registered")
+	}
+	if want := nodelock.NodeLockTimeout.String(); f.DefValue != want {
+		t.Errorf("node-lock-timeout default = %s, want %s (nodelock.NodeLockTimeout)", f.DefValue, want)
+	}
+	if config.NodeLockTimeout != nodelock.NodeLockTimeout {
+		t.Errorf("config.NodeLockTimeout = %v, want %v", config.NodeLockTimeout, nodelock.NodeLockTimeout)
+	}
+}